* thread.cc (pthread::precreate): Make sure mutex is recursive.

	Explain why.

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index ec7ff13da4a6851421be4771a6d6d8372fafc815..7b3d56d9ac6dbbbf28e92f90d3881e64714056de 100644 (file)
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,8 @@
+2012-05-23  Corinna Vinschen  <corinna@vinschen.de>
+
+       * thread.cc (pthread::precreate): Make sure mutex is recursive.
+       Explain why.
+
 2012-05-23  Corinna Vinschen  <corinna@vinschen.de>
 
        * thread.cc (pthread::pop_cleanup_handler): Move setting the cancelstate

diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index bb38ff3ed8d5c0c466b93cdfe8ce6a53003336ca..7a914b281d72a67319f415147fe365c42b1ca6a5 100644 (file)
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -431,8 +431,18 @@ pthread::precreate (pthread_attr *newattr)
       magic = 0;
       return;
     }
-  /* Change the mutex type to NORMAL to speed up mutex operations */
-  mutex.set_type (PTHREAD_MUTEX_NORMAL);
+  /* This mutex MUST be recursive.  Consider the following scenario:
+     - The thread installs a cleanup handler.
+     - The cleanup handler calls a function which itself installs a
+       cleanup handler.
+     - pthread_cancel is called for this thread.
+     - The thread's cleanup handler is called under mutex lock condition.
+     - The cleanup handler calls the subsequent function with cleanup handler.
+     - The function runs to completion, so it calls pthread_cleanup_pop.
+     - pthread_cleanup_pop calls pthread::pop_cleanup_handler which will again
+       try to lock the mutex.
+     - Deadlock. */
+  mutex.set_type (PTHREAD_MUTEX_RECURSIVE);
   if (!create_cancel_event ())
     magic = 0;
 }