Revert the reversion and go with implementation described in cgf-000017, with

some modifications.
* init.cc (dll_entry): Revert previous change.
* miscfuncs.cc: Include sigproc.h for exit_thread declaration.
* winsup.h (ExitThread): Define as 'exit_thread' to ensure no accidental use.
* 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.
* cygerrno.h (__set_errno): Define as extern so that no function code is ever
emitted.
* cygserver_ipc.h (cygserver_ipc.h): Ditto.
* miscfuncs.h (transform_chars): Ditto.
* path.h (has_attribute): Ditto.
* security.h (privilege_luid): Ditto.
* winsup.h (flush_file_buffers): Ditto.

13 files changed, 93 insertions, 23 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 4c6e05f32..7f3822f5d 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,5 +1,30 @@
 2012-12-21  Christopher Faylor  <me.cygwin2012@cgf.cx>
 
+	Revert the reversion and go with implementation described in
+	cgf-000017, with some modifications.
+	* init.cc (dll_entry): Revert previous change.
+	* miscfuncs.cc: Include sigproc.h for exit_thread declaration.
+	* winsup.h (ExitThread): Define as 'exit_thread' to ensure no
+	accidental use.
+	* 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.
+
+	* cygerrno.h (__set_errno): Define as extern so that no function code
+	is ever emitted.
+	* cygserver_ipc.h (cygserver_ipc.h): Ditto.
+	* miscfuncs.h (transform_chars): Ditto.
+	* path.h (has_attribute): Ditto.
+	* security.h (privilege_luid): Ditto.
+	* winsup.h (flush_file_buffers): Ditto.
+
+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.
diff --git a/winsup/cygwin/cygerrno.h b/winsup/cygwin/cygerrno.h
index 6f833f1a8..af7100648 100644
--- a/winsup/cygwin/cygerrno.h
+++ b/winsup/cygwin/cygerrno.h
@@ -22,7 +22,7 @@ int __stdcall geterrno_from_nt_status (NTSTATUS status, int deferrno = 13 /*EACC
 #define __seterrno_from_win_error(val) seterrno_from_win_error (__FILE__, __LINE__, val)
 #define __seterrno_from_nt_status(status) seterrno_from_nt_status (__FILE__, __LINE__, status)
 
-inline int
+extern inline int
 __set_errno (const char *fn, int ln, int val)
 {
   debug_printf ("%s:%d setting errno %d", fn, ln, val);
diff --git a/winsup/cygwin/cygserver_ipc.h b/winsup/cygwin/cygserver_ipc.h
index 4dbc8212f..d99f47647 100644
--- a/winsup/cygwin/cygserver_ipc.h
+++ b/winsup/cygwin/cygserver_ipc.h
@@ -33,7 +33,7 @@ struct proc {
 
 #ifdef __INSIDE_CYGWIN__
 #include "sigproc.h"
-inline void
+extern inline void
 ipc_set_proc_info (proc &blk)
 {
   blk.cygpid = getpid ();
diff --git a/winsup/cygwin/init.cc b/winsup/cygwin/init.cc
index d6cfb8868..1b245f581 100644
--- a/winsup/cygwin/init.cc
+++ b/winsup/cygwin/init.cc
@@ -13,7 +13,6 @@ details. */
 #include "cygtls.h"
 #include "ntdll.h"
 #include "shared_info.h"
-#include "sync.h"
 
 static DWORD _my_oldfunc;
 
@@ -96,14 +95,7 @@ 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);
-	  /* 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;
-	}
+	_my_tls.remove (0);
       /* 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 5f0625447..001461620 100644
--- a/winsup/cygwin/miscfuncs.cc
+++ b/winsup/cygwin/miscfuncs.cc
@@ -28,6 +28,7 @@ details. */
 #include "cygheap.h"
 #include "pinfo.h"
 #include "exception.h"
+#include "sigproc.h"
 
 long tls_ix = -1;
 
diff --git a/winsup/cygwin/miscfuncs.h b/winsup/cygwin/miscfuncs.h
index ff5fa1ef7..355fa103c 100644
--- a/winsup/cygwin/miscfuncs.h
+++ b/winsup/cygwin/miscfuncs.h
@@ -31,7 +31,7 @@ void slashify (const char *, char *, bool);
 #define isslash(c) ((c) == '/')
 
 extern void transform_chars (PWCHAR, PWCHAR);
-inline void
+extern inline void
 transform_chars (PUNICODE_STRING upath, USHORT start_idx)
 {
   transform_chars (upath->Buffer + start_idx,
diff --git a/winsup/cygwin/path.h b/winsup/cygwin/path.h
index a9e8b7c4a..4535c7e49 100644
--- a/winsup/cygwin/path.h
+++ b/winsup/cygwin/path.h
@@ -18,7 +18,7 @@ details. */
 #include <fcntl.h>
 #include <alloca.h>
 
-inline bool
+extern inline bool
 has_attribute (DWORD attributes, DWORD attribs_to_test)
 {
   return attributes != INVALID_FILE_ATTRIBUTES
diff --git a/winsup/cygwin/security.h b/winsup/cygwin/security.h
index 2ac101a69..8a132037b 100644
--- a/winsup/cygwin/security.h
+++ b/winsup/cygwin/security.h
@@ -354,13 +354,13 @@ extern cygpsid well_known_samba_unix_user_fake_sid;
 
 bool privilege_luid (const PWCHAR pname, LUID &luid, bool &high_integrity);
 
-inline BOOL
+extern inline BOOL
 well_known_sid_type (SID_NAME_USE type)
 {
   return type == SidTypeAlias || type == SidTypeWellKnownGroup;
 }
 
-inline BOOL
+extern inline BOOL
 legal_sid_type (SID_NAME_USE type)
 {
   return type == SidTypeUser  || type == SidTypeGroup
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..371641b40 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/winsup.h b/winsup/cygwin/winsup.h
index 9c1622f09..0638b44f2 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -261,12 +261,15 @@ enum mmap_region_status
 mmap_region_status mmap_is_attached_or_noreserve (void *addr, size_t len);
 bool is_mmapped_region (caddr_t start_addr, caddr_t end_address);
 
-inline bool flush_file_buffers (HANDLE h)
+extern inline bool flush_file_buffers (HANDLE h)
 {
   return (GetFileType (h) != FILE_TYPE_PIPE) ? FlushFileBuffers (h) : true;
 }
 #define FlushFileBuffers flush_file_buffers
 
+/* Make sure that regular ExitThread is never called */
+#define ExitThread exit_thread
+
 /**************************** Exports ******************************/
 
 extern "C" {