* DevNotes: Add entry cgf-000017.

* _cygtls.cc (_cygtls::call2): Use new exit_thread function in place of
ExitThread.
* miscfuncs.cc (thread_wrapper): Ditto.
* thread.cc (pthread::exit): Ditto.
(pthread_mutex::unlock): Set tid to NULL rather than 0.
(pthread_spinlock::unlock): Ditto.
* pinfo.cc (commune_process): Actually call lock_process constructor.
* sigproc.cc (exit_thread): New function.
(wait_sig): Handle __SIGTHREADEXIT case.  Don't just block rather than
returning from this function.
* sigproc.h (__SIGTHREADEXIT): New enum.
(exit_thread): Declare.
* sync.cc (muto::release): Accept a tls command-line argument.
* sync.h (muto::release): Accept a tls command-line parameter.  Default to
&_my_tls.

10 files changed, 106 insertions, 13 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index f1c444daf..76c6929ba 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,22 @@
+2012-12-21  Christopher Faylor  <me.cygwin2012@cgf.cx>
+
+	* DevNotes: Add entry cgf-000017.
+	* _cygtls.cc (_cygtls::call2): Use new exit_thread function in place of
+	ExitThread.
+	* miscfuncs.cc (thread_wrapper): Ditto.
+	* thread.cc (pthread::exit): Ditto.
+	(pthread_mutex::unlock): Set tid to NULL rather than 0.
+	(pthread_spinlock::unlock): Ditto.
+	* pinfo.cc (commune_process): Actually call lock_process constructor.
+	* sigproc.cc (exit_thread): New function.
+	(wait_sig): Handle __SIGTHREADEXIT case.  Don't just block rather than
+	returning from this function.
+	* sigproc.h (__SIGTHREADEXIT): New enum.
+	(exit_thread): Declare.
+	* sync.cc (muto::release): Accept a tls command-line argument.
+	* sync.h (muto::release): Accept a tls command-line parameter.  Default
+	to &_my_tls.
+
 2012-12-20  Corinna Vinschen  <corinna@vinschen.de>
 
 	* dcrt0.cc (build_argv): Allow quoted filenames in @ expression.
diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes
index aeca33076..efa338afd 100644
--- a/winsup/cygwin/DevNotes
+++ b/winsup/cygwin/DevNotes
@@ -1,3 +1,27 @@
+2012-12-21  cgf-000017
+
+The changes in this set are to work around the issue noted here:
+
+http://cygwin.com/ml/cygwin/2012-12/threads.html#00140
+
+The problem is, apparently, that the return value of an ExitThread()
+will take precedence over the return value of TerminateProcess/ExitProcess
+if the thread is the last one exiting.  That's rather amazing...
+
+For the fix, I replaced all calls to ExitThread with exit_thread().  The
+exit_thread function, creates a handle to the current thread and sends
+it to a packet via sig_send(__SIGTHREADEXIT).  Then it acquires the
+process lock and calls ExitThread.
+
+wait_sig will then wait for the handle, indicating that the thread has
+exited, and, when that has happened, remove the process lock on behalf
+of the now-defunct thread.  wait_sig will now also avoid actually
+exiting since it could trigger the same problem.
+
+Holding process_lock should prevent threads from exiting while a Cygwin
+process is shutting down.  They will just block forever in that case -
+just like wait_sig.
+
 2012-08-17  cgf-000016
 
 While debugging another problem I finally noticed that
diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index 2ff22ed98..a94f333a2 100644
--- a/winsup/cygwin/cygtls.cc
+++ b/winsup/cygwin/cygtls.cc
@@ -102,7 +102,7 @@ _cygtls::call2 (DWORD (*func) (void *, void *), void *arg, void *buf)
      dynamically loaded.  */
   if ((void *) func != (void *) dll_crt0_1
       && (void *) func != (void *) dll_dllcrt0_1)
-    ExitThread (res);
+    exit_thread (res);
 }
 
 void
diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc
index 5f0625447..3d76ed823 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -27,6 +27,7 @@ details. */
 #include "dtable.h"
 #include "cygheap.h"
 #include "pinfo.h"
+#include "sigproc.h"
 #include "exception.h"
 
 long tls_ix = -1;
@@ -546,7 +547,7 @@ thread_wrapper (VOID *arg)
 	   : : [WRAPPER_ARG] "r" (&wrapper_arg),
 	       [CYGTLS] "i" (CYGTLS_PADSIZE));
   /* Never return from here. */
-  ExitThread (0);
+  exit_thread (0);
 }
 
 HANDLE WINAPI
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 7280bce54..6bd05131b 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -515,7 +515,7 @@ commune_process (void *arg)
   if (process_sync)		// FIXME: this test shouldn't be necessary
     ProtectHandle (process_sync);
 
-  lock_process now ();
+  lock_process now;
   if (si._si_commune._si_code & PICOM_EXTRASTR)
     si._si_commune._si_str = (char *) (&si + 1);
 
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index a08a55ed3..be89d8685 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -553,6 +553,33 @@ sigproc_terminate (exit_states es)
     }
 }
 
+/* Exit the current thread very carefully.
+   See cgf-000017 in DevNotes for more details on why this is
+   necessary.  */
+void
+exit_thread (DWORD res)
+{
+  HANDLE h;
+
+  if (!DuplicateHandle (GetCurrentProcess (), GetCurrentThread (),
+                        GetCurrentProcess (), &h,
+                        0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+#ifdef DEBUGGING
+      system_printf ("couldn't duplicate the current thread, %E");
+#endif
+      ExitThread (res);
+    }
+  ProtectHandle1 (h, exit_thread);
+  siginfo_t si = {__SIGTHREADEXIT, SI_KERNEL};
+  si.si_value.sival_ptr = h;
+  /* Tell wait_sig to wait for this thread to exit.  It can then release
+     the lock below and close the above-opened handle. */
+  sig_send (myself_nowait, si, &_my_tls);
+  lock_process for_now;
+  ExitThread (0);	/* Should never hit this */
+}
+
 int __stdcall
 sig_send (_pinfo *p, int sig, _cygtls *tid)
 {
@@ -1419,6 +1446,23 @@ wait_sig (VOID *)
 	case __SIGSETPGRP:
 	  init_console_handler (true);
 	  break;
+	case __SIGTHREADEXIT:
+	  {
+	    /* Serialize thread exit as the thread exit code can be interpreted
+	       as the process exit code in some cases when racing with
+	       ExitProcess/TerminateProcess.
+	       So, wait for the thread which sent this signal to exit, then
+	       release the process lock which it held and close it's handle.
+	       See cgf-000017 in DevNotes for more details.
+	       */
+	    HANDLE h = (HANDLE) pack.si.si_value.sival_ptr;
+	    DWORD res = WaitForSingleObject (h, 5000);
+	    lock_process::force_release (pack.sigtls);
+	    ForceCloseHandle1 (h, exit_thread);
+	    if (res != WAIT_OBJECT_0)
+	      system_printf ("WaitForSingleObject(%p) for thread exit returned %u", h, res);
+	  }
+	  break;
 	default:
 	  if (pack.si.si_signo < 0)
 	    sig_clear (-pack.si.si_signo);
@@ -1461,5 +1505,8 @@ wait_sig (VOID *)
 
   close_my_readsig ();
   sigproc_printf ("signal thread exiting");
-  ExitThread (0);
+  /* Just wait for the process to go away.  Otherwise, this thread's
+     exit value could be interpreted as the process exit value.
+     See cgf-000017 in DevNotes for more details.  */
+  Sleep (INFINITE);
 }
diff --git a/winsup/cygwin/sigproc.h b/winsup/cygwin/sigproc.h
index a5a2e04c5..27f690dc1 100644
--- a/winsup/cygwin/sigproc.h
+++ b/winsup/cygwin/sigproc.h
@@ -25,7 +25,8 @@ enum
   __SIGHOLD	    = -(NSIG + 7),
   __SIGNOHOLD	    = -(NSIG + 8),
   __SIGEXIT	    = -(NSIG + 9),
-  __SIGSETPGRP	    = -(NSIG + 10)
+  __SIGSETPGRP	    = -(NSIG + 10),
+  __SIGTHREADEXIT   = -(NSIG + 11)
 };
 #endif
 
@@ -87,6 +88,7 @@ void __stdcall sigalloc ();
 
 int kill_pgrp (pid_t, siginfo_t&);
 int killsys (pid_t, int);
+void exit_thread (DWORD) __attribute__ ((regparm(1), noreturn));
 
 extern "C" void sigdelayed ();
 
diff --git a/winsup/cygwin/sync.cc b/winsup/cygwin/sync.cc
index f3796272f..d8b3d8f54 100644
--- a/winsup/cygwin/sync.cc
+++ b/winsup/cygwin/sync.cc
@@ -4,7 +4,8 @@
    which is intended to operate similarly to a mutex but attempts to
    avoid making expensive calls to the kernel.
 
-   Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2008, 2009, 2010 Red Hat, Inc.
+   Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2008, 2009, 2010, 2011, 2012
+   Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -109,10 +110,8 @@ muto::acquired ()
 
 /* Return the muto lock.  Needs to be called once per every acquire. */
 int
-muto::release ()
+muto::release (_cygtls *this_tls)
 {
-  void *this_tls = &_my_tls;
-
   if (tls != this_tls || !visits)
     {
       SetLastError (ERROR_NOT_OWNER);	/* Didn't have the lock. */
diff --git a/winsup/cygwin/sync.h b/winsup/cygwin/sync.h
index c9c5fb595..d424d39ab 100644
--- a/winsup/cygwin/sync.h
+++ b/winsup/cygwin/sync.h
@@ -33,7 +33,7 @@ public:
   ~muto ()
 #endif
   int acquire (DWORD ms = INFINITE) __attribute__ ((regparm (2))); /* Acquire the lock. */
-  int release () __attribute__ ((regparm (1)));		     /* Release the lock. */
+  int release (_cygtls * = &_my_tls) __attribute__ ((regparm (2))); /* Release the lock. */
 
   bool acquired () __attribute__ ((regparm (1)));
   void upforgrabs () {tls = this;}  // just set to an invalid address
@@ -60,6 +60,7 @@ public:
     if (!skip_unlock)
       locker.release ();
   }
+  static void force_release (_cygtls *tid) {locker.release (tid);}
   friend class dtable;
   friend class fhandler_fifo;
 };
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index eacf26741..415ad4af7 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -532,7 +532,7 @@ pthread::exit (void *value_ptr)
 	  _main_tls = dummy;
 	  _main_tls->initialized = false;
 	}
-      ExitThread (0);
+      exit_thread (0);
     }
 }
 
@@ -1778,7 +1778,7 @@ pthread_mutex::unlock ()
     {
       owner = (pthread_t) _unlocked_mutex;
 #ifdef DEBUGGING
-      tid = 0;
+      tid = NULL;
 #endif
       if (InterlockedDecrement ((long *) &lock_counter))
 	::SetEvent (win32_obj_id); // Another thread is waiting
@@ -1905,7 +1905,7 @@ pthread_spinlock::unlock ()
     {
       owner = (pthread_t) _unlocked_mutex;
 #ifdef DEBUGGING
-      tid = 0;
+      tid = NULL;
 #endif
       InterlockedExchange ((long *) &lock_counter, 0);
       ::SetEvent (win32_obj_id);