Re: bugs in gnulib thread modules

[Torvald Riegel]

On Thu, 2017-01-05 at 22:09 +0100, Bruno Haible wrote:
> Torvald Riegel wrote:
> > > Can you give line numbers, please? lib/glthread/lock.c and 
> > > lib/glthread/cond.c
> > > are large files. If you have found bugs there, *of course* I want to have
> > > them fixed.
> > 
> > I can understand both points.  However, I'd suggest that you first try
> > to find them (or that whoever maintains these files does); this is often
> > a good exercise, and helps make oneself sensitive for concurrency
> > problems.  If you don't want to do that first, I can give details.
> 
> We don't need to play "teacher and student" games here. I hold a 'Dr.',
> like you. I have experience in other areas than you. If I wasn't
> sensitive for concurrency problems, I wouldn't have introduced the
> 'lock' module in gnulib.
> 
> Can you please do me the favour and tell the line numbers of the two
> observations that you spotted? Thanks.

As you wish...

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().

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.

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.