Re: [PATCH]: gl_recursive_lock_init issue with pthread backend

[Yoann Vandoorselaere]

Le vendredi 07 décembre 2007 à 12:17 +0100, Bruno Haible a écrit : 
> Yoann Vandoorselaere wrote:
> > I'm worried about lock initialization. If initializing a mutex fail, the
> > application should be allowed to handle it gracefully. 
> 
> In all cases I know of, a lock initialization is nothing more than a memcpy.
> I'm not worried about it.

That is great, if most implementation initialize mutex using memcpy,
there is no room for failure. In this case, there is no problem checking
and returning an eventual error, in case one implementation does not
conform to this behavior.

> > Some applications provide threads support only to improve performance on
> > SMP capable hardware. If threads initialization fail, they should be
> > given a chance to revert to single thread capability.
> 
> Yes, but this is (or ought to be) handled inside the pthread_* functions.

Not sure what you mean here. An example of such application would launch
multiple threads in order to perform parallel computation. If for one
reason or another, initialization fail, it could revert to using a
single thread. 

> > I wouldn't like standard glibc pthread_mutex_lock() to call abort() for
> > me, would you? 
> 
> The glibc pthread_mutex_lock() calls assert() at 8 places.

Asserting is a good tool for catching obvious bug. The glibc 2.7
pthread_mutex_lock() function might return the following error from at
least 14 places:

- EAGAIN
- EDEADLK
- EOWNERDEAD
- ENOTRECOVERABLE
- EINVAL

This doesn't account for other function called by pthread_mutex_lock()
that might also trigger an error.

> > [0] POSIX define that pthread_mutex_lock() will return EAGAIN if the
> > "maximum number of recursive locks for mutex has been exceeded.", this
> > kind of error might be expected by the programmer.
> 
> glibc's intl/* code simply ignores this possibility. gnulib's 'lock' module
> is safer than this, it at least checks the return value.

I don't see any pthread use in glibc 2.7 intl code apart from the
intl/tst-gettext4.c and intl/tst-gettext5.c unit test, but anyway, other
applications might want to make use of this behavior.

> > you should let the application in control of the way it is
> > going to handle the issue (example: there might be important cleanup to
> > perform for persistant stored data to remain usable on next run).
> 
> That's an argument I can buy. I assume that such cleanup handlers are
> registered as C++ exceptions on the stack, or via atexit(). Can you
> suggest what to do instead of abort()?

Return the error, and let the application handle it, or not. 

If a developer does not care about handling pthread errors, nothing
prevent him to use something like:

assert(pthread_mutex_lock(&mutex) == 0);

We should let the developer define "how" such errors should be handled.

-- 
Yoann Vandoorselaere | Responsable R&D / CTO | PreludeIDS Technologies
Tel: +33 (0)8 70 70 21 58                  Fax: +33(0)4 78 42 21 58
http://www.prelude-ids.com