Re: bugs in gnulib thread modules

[Bruno Haible]

Torvald Riegel wrote:
> The double-checked locking in glthread_once_multithreaded is broken.  It
> has data races.  Remember the conversation we had about atomics?  Look
> at glibc's pthread_once to see what needs to be done.
> 
> A similar problem exists regarding the (use of the) initialized fields
> in the custom locks you implemented.  This is not a correct
> pthread_once, and it cannot reliably help you shield users from bad
> consequences due to not calling ..._init().

Good points, thanks a lot for these hints! Find attached the proposed fix.
It's gnulib's first use of <stdatomic.h>...

> I also think that glthread_rwlock_destroy_multithreaded should destroy
> the condvars first, then the mutex.  That may not be strictly required
> depending on how one interprets the wording around references to mutexes
> held by condvars.

I don't think the order matters, since the condvars must be empty at this
point. It's surely undefined behaviour to destroy an rwlock that still has
readers or writers pending (although POSIX [1] does not say so).
Anyway, patch attached.

> And you should not skip calling pthread_cond_destroy() and other
> destruction functions.  Doing so is not standards-conforming, in
> particular if you can ever reuse the memory for something else.  Not
> calling pthread_cond_destroy() will be broken when using current glibc,
> for example.

Yeah, I was a bit lazy in the error-handling case. Patch attached.

Bruno

[1] 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_destroy.html

0001-lock-Fix-data-races.patch
Description: Text Data

0002-lock-Improve-destruction.patch
Description: Text Data

0003-lock-Fix-resource-deallocation-upon-failure.patch
Description: Text Data