* cygthread.cc (cygthread::stub): Don't create event for long-running threads.

Initialize thread_sync event here which is used to Suspend using an event
rather than relying on SuspendThread/ResumeThread.
(cygthread::init): Save handle to runner thread for future termination.
(cygthread::cygthread): Only resume thread when it is actually suspended.
Otherwise signal thread completion event.
(cygthread::terminate): Forcibly terminate runner thread and any helper
threads.  Call DisableThreadLibrary calls if execing.
* cygthread.h (cygthread::thread_sync): Declare.
* dcrt0.cc (do_exit): Eliminate calls to obsolete window_terminate and
shared_terminate.
* exceptions.cc (events_terminate): Don't bother closing title_mutex since it
is going away anyway.
* pinfo.cc (_pinfo::exit): Call cygthread::terminate to ensure that threads are
shut down before process exit or otherwise strange races seem to occur.
* shared.cc (shared_terminate): Eliminate.
* shared.h (shared_terminate): Eliminate declaration.
* winsup.h (window_terminate): Eliminate declaration.
* spawn.cc (spawn_guts): Call cygthread::terminate early in process if execing.
Call DisableThreadLibrary calls if execing.
* window.cc (Winmain): Call ExitThread to force exit.
(window_terminate): Eliminate.
* dcrt0.cc (do_exit): Track exit state more closely.

11 files changed, 141 insertions, 77 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index f383b6d..c1a576f 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,31 @@
+2002-10-13  Christopher Faylor  <cgf@redhat.com>
+
+	* cygthread.cc (cygthread::stub): Don't create event for long-running
+	threads.  Initialize thread_sync event here which is used to Suspend
+	using an event rather than relying on SuspendThread/ResumeThread.
+	(cygthread::init): Save handle to runner thread for future termination.
+	(cygthread::cygthread): Only resume thread when it is actually
+	suspended.  Otherwise signal thread completion event.
+	(cygthread::terminate): Forcibly terminate runner thread and any helper
+	threads.  Call DisableThreadLibrary calls if execing.
+	* cygthread.h (cygthread::thread_sync): Declare.
+	* dcrt0.cc (do_exit): Eliminate calls to obsolete window_terminate and
+	shared_terminate.
+	* exceptions.cc (events_terminate): Don't bother closing title_mutex
+	since it is going away anyway.
+	* pinfo.cc (_pinfo::exit): Call cygthread::terminate to ensure that
+	threads are shut down before process exit or otherwise strange races
+	seem to occur.
+	* shared.cc (shared_terminate): Eliminate.
+	* shared.h (shared_terminate): Eliminate declaration.
+	* winsup.h (window_terminate): Eliminate declaration.
+	* spawn.cc (spawn_guts): Call cygthread::terminate early in process if
+	execing.  Call DisableThreadLibrary calls if execing.
+	* window.cc (Winmain): Call ExitThread to force exit.
+	(window_terminate): Eliminate.
+
+	* dcrt0.cc (do_exit): Track exit state more closely.
+
 2002-10-10  Christopher Faylor  <cgf@redhat.com>
 
 	* window.cc (gethwnd): Use SetThreadPriority method.
diff --git a/winsup/cygwin/cygthread.cc b/winsup/cygwin/cygthread.cc
index 74b35f4..ec217b7 100644
--- a/winsup/cygwin/cygthread.cc
+++ b/winsup/cygwin/cygthread.cc
@@ -25,24 +25,6 @@ int NO_COPY cygthread::initialized;
    per-thread initialization and loops waiting for new thread functions
    to execute.  */
 DWORD WINAPI
-cygthread::simplestub (VOID *arg)
-{
-  DECLARE_TLS_STORAGE;
-  exception_list except_entry;
-
-  /* Initialize this thread's ability to respond to things like
-     SIGSEGV or SIGFPE. */
-  init_exceptions (&except_entry);
-
-  cygthread *info = (cygthread *) arg;
-  info->func (info->arg == cygself ? info : info->arg);
-  ExitThread (0);
-}
-
-/* Initial stub called by cygthread constructor. Performs initial
-   per-thread initialization and loops waiting for new thread functions
-   to execute.  */
-DWORD WINAPI
 cygthread::stub (VOID *arg)
 {
   DECLARE_TLS_STORAGE;
@@ -54,12 +36,15 @@ cygthread::stub (VOID *arg)
 
   cygthread *info = (cygthread *) arg;
   if (info->arg == cygself)
-    info->ev = NULL;
+    info->ev = info->thread_sync = NULL;
   else
-    info->ev = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+    {
+      info->ev = CreateEvent (&sec_none_nih, TRUE, FALSE, NULL);
+      info->thread_sync = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
+    }
   while (1)
     {
-      if (!info->func)
+      if (!info->func || initialized < 0)
 	ExitThread (0);
 
       /* Cygwin threads should not call ExitThread directly */
@@ -71,13 +56,34 @@ cygthread::stub (VOID *arg)
 #endif
       SetEvent (info->ev);
       info->__name = NULL;
-      if (initialized >= 0)
-	SuspendThread (info->h);
-      else
-	ExitThread (0);
+      switch (WaitForSingleObject (info->thread_sync, INFINITE))
+	{
+	case WAIT_OBJECT_0:
+	  continue;
+	default:
+	  api_fatal ("WFSO failed, %E");
+	  break;
+	}
     }
 }
 
+/* Overflow stub called by cygthread constructor. Calls specified function
+   and then exits the thread.  */
+DWORD WINAPI
+cygthread::simplestub (VOID *arg)
+{
+  DECLARE_TLS_STORAGE;
+  exception_list except_entry;
+
+  /* Initialize this thread's ability to respond to things like
+     SIGSEGV or SIGFPE. */
+  init_exceptions (&except_entry);
+
+  cygthread *info = (cygthread *) arg;
+  info->func (info->arg == cygself ? info : info->arg);
+  ExitThread (0);
+}
+
 /* This function runs in a secondary thread and starts up a bunch of
    other suspended threads for use in the cygthread pool. */
 DWORD WINAPI
@@ -95,15 +101,16 @@ cygthread::runner (VOID *arg)
   ExitThread (0);
 }
 
+HANDLE NO_COPY runner_handle;
+DWORD NO_COPY runner_tid;
 /* Start things going.  Called from dll_crt0_1. */
 void
 cygthread::init ()
 {
-  DWORD tid;
-  HANDLE h = CreateThread (&sec_none_nih, 0, cygthread::runner, NULL, 0, &tid);
-  if (!h)
+  runner_handle = CreateThread (&sec_none_nih, 0, cygthread::runner, NULL, 0,
+				&runner_tid);
+  if (!runner_handle)
     api_fatal ("can't start thread_runner, %E");
-  CloseHandle (h);
   main_thread_id = GetCurrentThreadId ();
 }
 
@@ -179,7 +186,7 @@ cygthread::cygthread (LPTHREAD_START_ROUTINE start, LPVOID param,
     api_fatal ("name should never be NULL");
 #endif
   thread_printf ("name %s, id %p", name, id);
-  while (!h || ResumeThread (h) != 1)
+  while (!h)
 #ifndef DEBUGGING
     Sleep (0);
 #else
@@ -191,6 +198,10 @@ cygthread::cygthread (LPTHREAD_START_ROUTINE start, LPVOID param,
   __name = name;	/* Need to set after thread has woken up to
 			   ensure that it won't be cleared by exiting
 			   thread. */
+  if (thread_sync)
+    SetEvent (thread_sync);
+  else
+    ResumeThread (h);
 }
 
 /* Return the symbolic name of the current thread for debugging.
@@ -241,7 +252,7 @@ cygthread::exit_thread ()
 }
 
 /* Detach the cygthread from the current thread.  Note that the
-   theory is that cygthread's are only associated with one thread.
+   theory is that cygthreads are only associated with one thread.
    So, there should be no problems with multiple threads doing waits
    on the one cygthread. */
 void
@@ -280,9 +291,32 @@ cygthread::detach ()
 void
 cygthread::terminate ()
 {
-  initialized = -1;
-  /* Signal the event for all running threads */
-  for (cygthread *info = threads + NTHREADS - 1; info >= threads; info--)
-    if (!InterlockedExchange ((LPLONG) &info->avail, 0) && info->ev)
-      SetEvent (info->ev);
+  /* Wow.  All of this seems to be necessary or (on Windows 9x at least) the
+     process will sometimes deadlock if there are suspended threads.  I assume
+     that something funky is happening like a suspended thread being created
+     while the process is exiting or something.  In particular, it seems like
+     the WaitForSingleObjects are necessary since it appears that the
+     TerminateThread call may happen asynchronously, i.e., when TerminateThread
+     returns, the thread may not yet have terminated. */
+  if (runner_handle && initialized >= 0)
+    {
+      /* Don't care about detaching (or attaching) threads now */
+      if (cygwin_hmodule && !DisableThreadLibraryCalls (cygwin_hmodule))
+	system_printf ("DisableThreadLibraryCalls (%p) failed, %E",
+		       cygwin_hmodule);
+      initialized = -1;
+      (void) TerminateThread (runner_handle, 0);
+      (void) WaitForSingleObject (runner_handle, INFINITE);
+      (void) CloseHandle (runner_handle);
+      for (unsigned i = 0; i < NTHREADS; i++)
+	if (threads[i].h)
+	  {
+	    TerminateThread (threads[i].h, 0);
+	    (void) WaitForSingleObject (threads[i].h, INFINITE);
+	    (void) CloseHandle (threads[i].h);
+#ifdef DEBUGGING
+	    threads[i].h = NULL;
+#endif
+	  }
+    }
 }
diff --git a/winsup/cygwin/cygthread.h b/winsup/cygwin/cygthread.h
index 5ea74b3..ded205f 100644
--- a/winsup/cygwin/cygthread.h
+++ b/winsup/cygwin/cygthread.h
@@ -12,6 +12,7 @@ class cygthread
   DWORD id;
   HANDLE h;
   HANDLE ev;
+  HANDLE thread_sync;
   const char *__name;
   LPTHREAD_START_ROUTINE func;
   VOID *arg;
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index f55b26d..36442fc 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -945,19 +945,26 @@ __main (void)
   do_global_ctors (user_data->ctors, FALSE);
 }
 
-enum
+enum exit_states
   {
-    ES_THREADTERM = 1,
-    ES_SIGNAL = 2,
-    ES_CLOSEALL = 3,
-    ES_SIGPROCTERMINATE = 4
+    ES_NOT_EXITING = 0,
+    ES_THREADTERM,
+    ES_SIGNAL,
+    ES_CLOSEALL,
+    ES_SIGPROCTERMINATE,
+    ES_TITLE,
+    ES_HUP_PGRP,
+    ES_HUP_SID,
+    ES_TTY_TERMINATE,
+    ES_EVENTS_TERMINATE
   };
 
+exit_states NO_COPY exit_state;
+
 extern "C" void __stdcall
 do_exit (int status)
 {
   UINT n = (UINT) status;
-  static int NO_COPY exit_state = 0;
 
   syscall_printf ("do_exit (%d)", n);
 
@@ -965,10 +972,6 @@ do_exit (int status)
   if (vf != NULL && vf->pid < 0)
     vf->restore_exit (status);
 
-  if (!DisableThreadLibraryCalls (cygwin_hmodule))
-    system_printf ("DisableThreadLibraryCalls (%p) failed, %E",
-		   cygwin_hmodule);
-
   if (exit_state < ES_THREADTERM)
     {
       exit_state = ES_THREADTERM;
@@ -999,16 +1002,18 @@ do_exit (int status)
       sigproc_terminate ();
     }
 
-  if (n & EXIT_REPARENTING)
-    n &= ~EXIT_REPARENTING;
-  else
+  myself->stopsig = 0;
+  if (exit_state < ES_TITLE)
     {
-      myself->stopsig = 0;
-
+      exit_state = ES_TITLE;
       /* restore console title */
       if (old_title && display_title)
 	set_console_title (old_title);
+    }
 
+  if (exit_state < ES_HUP_PGRP)
+    {
+      exit_state = ES_HUP_PGRP;
       /* Kill orphaned children on group leader exit */
       if (myself->has_pgid_children && myself->pid == myself->pgid)
 	{
@@ -1016,7 +1021,11 @@ do_exit (int status)
 			  myself->pid, myself->pgid);
 	  kill_pgrp (myself->pgid, -SIGHUP);
 	}
+    }
 
+  if (exit_state < ES_HUP_SID)
+    {
+      exit_state = ES_HUP_SID;
       /* Kill the foreground process group on session leader exit */
       if (getpgrp () > 0 && myself->pid == myself->sid && real_tty_attached (myself))
 	{
@@ -1029,12 +1038,19 @@ do_exit (int status)
 	    tp->kill_pgrp (SIGHUP);
 	}
 
+    }
+
+  if (exit_state < ES_TTY_TERMINATE)
+    {
+      exit_state = ES_TTY_TERMINATE;
       tty_terminate ();
     }
 
-  window_terminate ();
-  events_terminate ();
-  shared_terminate ();
+  if (exit_state < ES_EVENTS_TERMINATE)
+    {
+      exit_state = ES_EVENTS_TERMINATE;
+      events_terminate ();
+    }
 
   minimal_printf ("winpid %d, exit %d", GetCurrentProcessId (), n);
   myself->exit (n);
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 0c8758d..9854c10 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1136,7 +1136,6 @@ events_init (void)
 void
 events_terminate (void)
 {
-  ForceCloseHandle (title_mutex);
   exit_already = 1;
 }
 
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 4591fed..e9716de 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -26,6 +26,7 @@ details. */
 #include <assert.h>
 #include <ntdef.h>
 #include "ntdll.h"
+#include "cygthread.h"
 
 static char NO_COPY pinfo_dummy[sizeof (_pinfo)] = {0};
 
@@ -109,6 +110,7 @@ _pinfo::exit (UINT n, bool norecord)
       add_rusage (&rusage_self, &r);
     }
 
+  cygthread::terminate ();
   sigproc_printf ("Calling ExitProcess %d", n);
   ExitProcess (n);
 }
diff --git a/winsup/cygwin/shared.cc b/winsup/cygwin/shared.cc
index 5c3beb9..1319a88 100644
--- a/winsup/cygwin/shared.cc
+++ b/winsup/cygwin/shared.cc
@@ -170,15 +170,6 @@ memory_init ()
 
 }
 
-void __stdcall
-shared_terminate ()
-{
-  if (cygheap->shared_h)
-    ForceCloseHandle (cygheap->shared_h);
-  if (cygwin_mount_h)
-    ForceCloseHandle (cygwin_mount_h);
-}
-
 unsigned
 shared_info::heap_chunk_size ()
 {
diff --git a/winsup/cygwin/shared_info.h b/winsup/cygwin/shared_info.h
index 945bc17..2da3293 100644
--- a/winsup/cygwin/shared_info.h
+++ b/winsup/cygwin/shared_info.h
@@ -162,7 +162,6 @@ extern mount_info *mount_table;
 extern HANDLE cygwin_mount_h;
 
 void __stdcall memory_init (void);
-void __stdcall shared_terminate (void);
 
 #define shared_align_past(p) \
   ((char *) (system_info.dwAllocationGranularity * \
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 34927c1..b3bf304 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -34,6 +34,7 @@ details. */
 #include "perthread.h"
 #include "registry.h"
 #include "environ.h"
+#include "cygthread.h"
 
 #define LINE_BUF_CHUNK (MAX_PATH * 2)
 
@@ -715,12 +716,13 @@ spawn_guts (const char * prog_arg, const char *const *argv,
       cygheap_setup_for_child_cleanup (newheap, &ciresrv, 1);
       if (mode == _P_OVERLAY)
 	ResumeThread (pi.hThread);
+      cygthread::terminate ();
     }
 
-  if (mode == _P_OVERLAY)
-    cygpid = myself->pid;
-  else
+  if (mode != _P_OVERLAY)
     cygpid = cygwin_pid (pi.dwProcessId);
+  else
+    cygpid = myself->pid;
 
   /* We print the original program name here so the user can see that too.  */
   syscall_printf ("%d = spawn_guts (%s, %.132s)",
diff --git a/winsup/cygwin/window.cc b/winsup/cygwin/window.cc
index 7c67fae..80e95a1 100644
--- a/winsup/cygwin/window.cc
+++ b/winsup/cygwin/window.cc
@@ -121,7 +121,7 @@ Winmain (VOID *)
   while (GetMessage (&msg, ourhwnd, 0, 0) == TRUE)
     DispatchMessage (&msg);
 
-  return msg.wParam;
+  ExitThread (0);
 }
 
 HWND __stdcall
@@ -141,13 +141,6 @@ gethwnd ()
   return ourhwnd;
 }
 
-void __stdcall
-window_terminate ()
-{
-  if (ourhwnd)
-    SendMessage (ourhwnd, WM_DESTROY, 0, 0);
-}
-
 extern "C" int
 setitimer (int which, const struct itimerval *value, struct itimerval *oldvalue)
 {
diff --git a/winsup/cygwin/winsup.h b/winsup/cygwin/winsup.h
index 1aebfd7..4b2344c 100644
--- a/winsup/cygwin/winsup.h
+++ b/winsup/cygwin/winsup.h
@@ -169,7 +169,6 @@ BOOL __stdcall check_pty_fds (void);
 
 /* Invisible window initialization/termination. */
 HWND __stdcall gethwnd (void);
-void __stdcall window_terminate (void);
 
 /* Globals that handle initialization of winsock in a child process. */
 extern HANDLE wsock32_handle;