[PATCH] Pthread fixes arising.

[Dave Korn]

    Hi everyone,

  The attached patch fixes the pthread bugs discovered and reported between
Thomas Stalder and myself.  The winbase.h inline asms bug has already been the
subject of a long thread.

  The latest fix is a simple racy optimisation in __cygwin_lock_{,un}lock;
they attempt to save themselves the bother of locking if multiple threads
aren't in operation, but when you have threads asynchronously being born and
dying, this can mean that the number of locks and unlocks becomes mismatched,
as the optimisation decision can be taken differently for both halves of a
matching lock/unlock pair of calls.  Here's an example of it going wrong in
practice:

   84   47795 [main] stalder3 3612 __cygwin_lock_lock: threadcount 2.  locking
 4981   47948 [main] stalder3 3612 __cygwin_lock_lock: threadcount 2.  locking

   ********* other thread dies here ************

-4876   43704 [main] stalder3 3612 __cygwin_lock_unlock: threadcount 1.  not
unlocking
   48   43752 [main] stalder3 3612 __cygwin_lock_unlock: threadcount 1.  not
unlocking

   ******** main thinks it has unlocked the lock but it still holds it *****

-4874   44571 [main] stalder3 3612 __cygwin_lock_lock: threadcount 2.  locking
 4982   49599 [main] stalder3 3612 __cygwin_lock_lock: threadcount 2.  locking
 4968   50294 [main] stalder3 3612 __cygwin_lock_unlock: threadcount 2.  unlocked
 4969   50398 [main] stalder3 3612 __cygwin_lock_unlock: threadcount 2.  unlocked

  *** main is now taking and releasing it recursively with a bias of one ****

  105   50554 [unknown (0x7F4)] stalder3 3612 __cygwin_lock_lock: threadcount
2.  locking

  Now this thread waits potentially forever to take the lock.  But while it is
doing this, it happens to be holding the user-level mutex.  As soon as the
main thread next waits on this mutex, while still recursively holding the
__cygwin_lock_* mutex, we have a classic deadlock caused by priority
inversion.  I don't figure the optimisation is worth the trouble - in theory
we could perhaps still try and hang onto it from program startup until the
first user thread gets created and then make sure it never gets re-enabled for
the rest of the runtime even if the number of threads drops back down again,
and the worst that would happen is excess unlock calls, but even that seems a
bit ugly.  So I dropped it altogether.

winsup/cygwin/ChangeLog

	* thread.cc (__cygwin_lock_lock):  Delete racy optimisation.
	(__cygwin_lock_unlock):  Likewise.

	* winbase.h (ilockexch):  Fix asm constraints.
	(ilockcmpexch):  Likewise.

  OK for head?

    cheers,
      DaveK


-------------- next part --------------
A non-text attachment was scrubbed...
Name: pthread-fixes.diff
Type: text/x-c
Size: 2598 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20090603/993376dc/attachment.bin>