* DevNotes: Add entry cgf-000018.

* init.cc (dll_entry): Grab process lock before exiting to ensure that thread
doesn't exit before parent if parent is exiting.
* _cygtls.cc (_cygtls::call2): Revert previous 2012-12-21 change.
* miscfuncs.cc (thread_wrapper): Ditto.
* thread.cc (pthread::exit): Ditto.
* sigproc.cc (exit_thread): Ditto.
(wait_sig): Ditto.
* sync.cc (muto::release): Ditto.
* sync.h (muto::release): Ditto.
* sigproc.h (__SIGTHREADEXIT): Delete enum.
(exit_thread): Delete declaration.

10 files changed, 45 insertions, 61 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 76c6929ba..4c6e05f32 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,20 @@
 2012-12-21  Christopher Faylor  <me.cygwin2012@cgf.cx>
 
+	* DevNotes: Add entry cgf-000018.
+	* init.cc (dll_entry): Grab process lock before exiting to ensure that
+	thread doesn't exit before parent if parent is exiting.
+	* _cygtls.cc (_cygtls::call2): Revert previous 2012-12-21 change.
+	* miscfuncs.cc (thread_wrapper): Ditto.
+	* thread.cc (pthread::exit): Ditto.
+	* sigproc.cc (exit_thread): Ditto.
+	(wait_sig): Ditto.
+	* sync.cc (muto::release): Ditto.
+	* sync.h (muto::release): Ditto.
+	* sigproc.h (__SIGTHREADEXIT): Delete enum.
+	(exit_thread): Delete declaration.
+
+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.
diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes
index efa338afd..04abd99cc 100644
--- a/winsup/cygwin/DevNotes
+++ b/winsup/cygwin/DevNotes
@@ -1,3 +1,14 @@
+2012-12-21  cgf-000018
+
+Re: cgf-000017
+
+It occurred to me that just getting the process lock during
+DLL_THREAD_DETACH in dll_entry() might be adequate to fix this
+problem.  It's certainly much less intrusive.
+
+There are potential deadlock problems with grabbing a lock in
+this code, though, so this check-in will be experimental.
+
 2012-12-21  cgf-000017
 
 The changes in this set are to work around the issue noted here:
diff --git a/winsup/cygwin/cygtls.cc b/winsup/cygwin/cygtls.cc
index a94f333a2..2ff22ed98 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)
-    exit_thread (res);
+    ExitThread (res);
 }
 
 void
diff --git a/winsup/cygwin/init.cc b/winsup/cygwin/init.cc
index 1b245f581..d6cfb8868 100644
--- a/winsup/cygwin/init.cc
+++ b/winsup/cygwin/init.cc
@@ -13,6 +13,7 @@ details. */
 #include "cygtls.h"
 #include "ntdll.h"
 #include "shared_info.h"
+#include "sync.h"
 
 static DWORD _my_oldfunc;
 
@@ -95,7 +96,14 @@ dll_entry (HANDLE h, DWORD reason, void *static_load)
       if (dll_finished_loading
 	  && (PVOID) &_my_tls > (PVOID) &test_stack_marker
 	  && _my_tls.isinitialized ())
-	_my_tls.remove (0);
+	{
+	  _my_tls.remove (0);
+	  /* Make sure that we don't exit until the process has exited.
+	     Otherwise there is a potential race where the thread exit
+	     code could be considered to be the process exit code.
+	     See cgf-000017 and cgf-000018 in DevNotes.  */
+	  lock_process here;
+	}
       /* Windows 2000 has a bug in NtTerminateThread.  Instead of releasing
 	 the stack at teb->DeallocationStack it uses the value of
 	 teb->Tib.StackLimit to evaluate the stack address.  So we just claim
diff --git a/winsup/cygwin/miscfuncs.cc b/winsup/cygwin/miscfuncs.cc
index 3d76ed823..5f0625447 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -27,7 +27,6 @@ details. */
 #include "dtable.h"
 #include "cygheap.h"
 #include "pinfo.h"
-#include "sigproc.h"
 #include "exception.h"
 
 long tls_ix = -1;
@@ -547,7 +546,7 @@ thread_wrapper (VOID *arg)
 	   : : [WRAPPER_ARG] "r" (&wrapper_arg),
 	       [CYGTLS] "i" (CYGTLS_PADSIZE));
   /* Never return from here. */
-  exit_thread (0);
+  ExitThread (0);
 }
 
 HANDLE WINAPI
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index be89d8685..a08a55ed3 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -553,33 +553,6 @@ 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)
 {
@@ -1446,23 +1419,6 @@ 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);
@@ -1505,8 +1461,5 @@ wait_sig (VOID *)
 
   close_my_readsig ();
   sigproc_printf ("signal thread exiting");
-  /* 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);
+  ExitThread (0);
 }
diff --git a/winsup/cygwin/sigproc.h b/winsup/cygwin/sigproc.h
index 27f690dc1..a5a2e04c5 100644
--- a/winsup/cygwin/sigproc.h
+++ b/winsup/cygwin/sigproc.h
@@ -25,8 +25,7 @@ enum
   __SIGHOLD	    = -(NSIG + 7),
   __SIGNOHOLD	    = -(NSIG + 8),
   __SIGEXIT	    = -(NSIG + 9),
-  __SIGSETPGRP	    = -(NSIG + 10),
-  __SIGTHREADEXIT   = -(NSIG + 11)
+  __SIGSETPGRP	    = -(NSIG + 10)
 };
 #endif
 
@@ -88,7 +87,6 @@ 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 d8b3d8f54..f3796272f 100644
--- a/winsup/cygwin/sync.cc
+++ b/winsup/cygwin/sync.cc
@@ -4,8 +4,7 @@
    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, 2011, 2012
-   Red Hat, Inc.
+   Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2008, 2009, 2010 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -110,8 +109,10 @@ muto::acquired ()
 
 /* Return the muto lock.  Needs to be called once per every acquire. */
 int
-muto::release (_cygtls *this_tls)
+muto::release ()
 {
+  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 d424d39ab..c9c5fb595 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 (_cygtls * = &_my_tls) __attribute__ ((regparm (2))); /* Release the lock. */
+  int release () __attribute__ ((regparm (1)));		     /* Release the lock. */
 
   bool acquired () __attribute__ ((regparm (1)));
   void upforgrabs () {tls = this;}  // just set to an invalid address
@@ -60,7 +60,6 @@ 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 415ad4af7..187ea52fd 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;
 	}
-      exit_thread (0);
+      ExitThread (0);
     }
 }