* dcrt0.cc (child_info_fork::alloc_stack_hard_way): Change sense of guard test.

Increase size of stack reserved and increase size before the current stack
pointer.  Use pointers when doing arithmetic.
(dll_crt0_1): Initialize exception handler when we notice we're the child of a
fork from non-main thread.
* fork.cc (frok::parent): Make argument volatile.
(frok::child): Ditto.
(lock_signals): New class.
(lock_pthread): Ditto.
(hold_everhthing): Ditto.
(frok::parent): Move atforkprepare and atforkparent to lock_pthread class.
(fork): Make ischild boolean.  Use hold_everything variable within limited
scope to set various mutexes in such a way as to avoid deadlocks.
* thread.h (pthread_mutex::tid): New variable, active when debugging for
tracking thread id of owner.
(pthread_mutex::set_owner): Set tid when debugging.
* thread.cc (pthread_mutex::pthread_mutex): Clear tid.
(pthread_mutex::_unlock): Ditto when unlocking.
(pthread_mutex::fixup_after_fork): Set tid to special value after forking since
owner is unknown.

5 files changed, 171 insertions, 55 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 6337ff34b..37f3dd9f0 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,28 @@
+2007-02-22  Christopher Faylor  <me@cgf.cx>
+
+	* dcrt0.cc (child_info_fork::alloc_stack_hard_way): Change sense of
+	guard test.  Increase size of stack reserved and increase size before
+	the current stack pointer.  Use pointers when doing arithmetic.
+	(dll_crt0_1): Initialize exception handler when we notice we're the
+	child of a fork from non-main thread.
+	* fork.cc (frok::parent): Make argument volatile.
+	(frok::child): Ditto.
+	(lock_signals): New class.
+	(lock_pthread): Ditto.
+	(hold_everhthing): Ditto.
+	(frok::parent): Move atforkprepare and atforkparent to lock_pthread
+	class.
+	(fork): Make ischild boolean.  Use hold_everything variable within
+	limited scope to set various mutexes in such a way as to avoid
+	deadlocks.
+	* thread.h (pthread_mutex::tid): New variable, active when debugging
+	for tracking thread id of owner.
+	(pthread_mutex::set_owner): Set tid when debugging.
+	* thread.cc (pthread_mutex::pthread_mutex): Clear tid.
+	(pthread_mutex::_unlock): Ditto when unlocking.
+	(pthread_mutex::fixup_after_fork): Set tid to special value after
+	forking since owner is unknown.
+
 2007-02-22  Corinna Vinschen  <corinna@vinschen.de>
 
 	Throughout replace all usage of wincap.shared with the constant
diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index c31bbb72f..57c93017c 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -451,8 +451,7 @@ check_sanity_and_sync (per_process *p)
 
 child_info NO_COPY *child_proc_info = NULL;
 
-#define CYGWIN_GUARD ((wincap.has_page_guard ()) ? \
-		     PAGE_EXECUTE_READWRITE|PAGE_GUARD : PAGE_NOACCESS)
+#define CYGWIN_GUARD (PAGE_EXECUTE_READWRITE | PAGE_GUARD)
 
 void
 child_info_fork::alloc_stack_hard_way (volatile char *b)
@@ -461,7 +460,7 @@ child_info_fork::alloc_stack_hard_way (volatile char *b)
   MEMORY_BASIC_INFORMATION m;
   void *newbase;
   int newlen;
-  bool noguard;
+  bool guard;
 
   if (!VirtualQuery ((LPCVOID) &b, &m, sizeof m))
     api_fatal ("fork: couldn't get stack info, %E");
@@ -472,28 +471,29 @@ child_info_fork::alloc_stack_hard_way (volatile char *b)
     {
       newbase = curbot;
       newlen = (LPBYTE) stackbottom - (LPBYTE) curbot;
-      noguard = 1;
+      guard = false;
     }
   else
     {
-      newbase = stacktop;
-      newlen = (DWORD) stackbottom - (DWORD) stacktop;
-      noguard = 0;
+      newbase = (LPBYTE) stacktop - (128 * 1024);
+      newlen = (LPBYTE) stackbottom - (LPBYTE) newbase;
+      guard = true;
     }
+
   if (!VirtualAlloc (newbase, newlen, MEM_RESERVE, PAGE_NOACCESS))
     api_fatal ("fork: can't reserve memory for stack %p - %p, %E",
 		stacktop, stackbottom);
-
-  new_stack_pointer = (void *) ((LPBYTE) stackbottom - stacksize);
+  new_stack_pointer = (void *) ((LPBYTE) stackbottom - (stacksize += 8192));
   if (!VirtualAlloc (new_stack_pointer, stacksize, MEM_COMMIT,
 		     PAGE_EXECUTE_READWRITE))
     api_fatal ("fork: can't commit memory for stack %p(%d), %E",
 	       new_stack_pointer, stacksize);
   if (!VirtualQuery ((LPCVOID) new_stack_pointer, &m, sizeof m))
     api_fatal ("fork: couldn't get new stack info, %E");
-  if (!noguard)
+
+  if (guard)
     {
-      m.BaseAddress = (LPVOID) ((DWORD) m.BaseAddress - 1);
+      m.BaseAddress = (LPBYTE) m.BaseAddress - 1;
       if (!VirtualAlloc ((LPVOID) m.BaseAddress, 1, MEM_COMMIT,
 			 CYGWIN_GUARD))
 	api_fatal ("fork: couldn't allocate new stack guard page %p, %E",
@@ -826,7 +826,9 @@ dll_crt0_1 (void *)
 	{
 	  _tlsbase = (char *) fork_info->stackbottom;
 	  _tlstop = (char *) fork_info->stacktop;
+	  _my_tls.init_exception_handler (_cygtls::handle_exceptions);
 	}
+
       longjmp (fork_info->jmp, true);
     }
 
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 51afce9c1..f7949d36d 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -45,11 +45,87 @@ class frok
   const char *error;
   int child_pid;
   int this_errno;
-  int __stdcall parent (void *esp);
-  int __stdcall child (void *esp);
+  int __stdcall parent (volatile char * volatile here);
+  int __stdcall child (volatile char * volatile here);
   friend int fork ();
 };
 
+class lock_signals
+{
+  bool worked;
+public:
+  lock_signals ()
+  {
+    worked = sig_send (NULL, __SIGHOLD) == 0;
+  }
+  operator int () const
+  {
+    return worked;
+  }
+  void dont_bother ()
+  {
+    worked = false;
+  }
+  ~lock_signals ()
+  {
+    if (worked)
+      sig_send (NULL, __SIGNOHOLD);
+  }
+};
+
+class lock_pthread
+{
+  bool bother;
+public:
+  lock_pthread (): bother (1)
+  {
+    pthread::atforkprepare ();
+  }
+  void dont_bother ()
+  {
+    bother = false;
+  }
+  ~lock_pthread ()
+  {
+    if (bother)
+      pthread::atforkparent ();
+  }
+};
+
+class hold_everything
+{
+  bool& ischild;
+  /* Note the order of the locks below.  It is important,
+     to avoid races, that the lock order be preserved.
+     
+     pthread is first because it serves as a master lock
+     against other forks being attempted while this one is active.
+     
+     signals is next to stop signal processing for the duration
+     of the fork.
+
+     process is last.  If it is put before signals, then a deadlock
+     could be introduced if the process attempts to exit due to a signal. */
+  lock_pthread pthread;
+  lock_signals signals;
+  lock_process process;
+
+public:
+  hold_everything (bool& x): ischild (x) {}
+  operator int () const {return signals;}
+
+  ~hold_everything()
+  {
+    if (ischild)
+      {
+	pthread.dont_bother ();
+	process.dont_bother ();
+	signals.dont_bother ();
+      }
+  }
+
+};
+
 static void
 resume_child (HANDLE forker_finished)
 {
@@ -92,7 +168,7 @@ sync_with_parent (const char *s, bool hang_self)
 }
 
 int __stdcall
-frok::child (void *)
+frok::child (volatile char * volatile here)
 {
   HANDLE& hParent = ch.parent;
   extern void fixup_hooks_after_fork ();
@@ -213,7 +289,7 @@ slow_pid_reuse (HANDLE h)
 #endif
 
 int __stdcall
-frok::parent (void *stack_here)
+frok::parent (volatile char * volatile stack_here)
 {
   HANDLE forker_finished;
   DWORD rc;
@@ -224,8 +300,6 @@ frok::parent (void *stack_here)
   pinfo child;
   static char errbuf[256];
 
-  pthread::atforkprepare ();
-
   int c_flags = GetPriorityClass (hMainProc);
   debug_printf ("priority class %d", c_flags);
 
@@ -271,7 +345,7 @@ frok::parent (void *stack_here)
   ch.forker_finished = forker_finished;
 
   ch.stackbottom = _tlsbase;
-  ch.stacktop = stack_here;
+  ch.stacktop = (void *) stack_here;
   ch.stacksize = (char *) ch.stackbottom - (char *) stack_here;
   debug_printf ("stack - bottom %p, top %p, size %d",
 		ch.stackbottom, ch.stacktop, ch.stacksize);
@@ -492,7 +566,6 @@ frok::parent (void *stack_here)
 
   ForceCloseHandle (forker_finished);
   forker_finished = NULL;
-  pthread::atforkparent ();
 
   return child_pid;
 
@@ -516,14 +589,13 @@ extern "C" int
 fork ()
 {
   frok grouped;
-  MALLOC_CHECK;
 
   debug_printf ("entering");
   grouped.first_dll = NULL;
   grouped.load_dlls = 0;
 
   int res;
-  int ischild;
+  bool ischild = false;
 
   myself->set_has_pgid_children ();
 
@@ -535,30 +607,28 @@ fork ()
       return -1;
     }
 
-  lock_process now;
-  if (sig_send (NULL, __SIGHOLD))
-    {
-      if (exit_state)
-	Sleep (INFINITE);
-      set_errno (EAGAIN);
-      return -1;
-    }
+  {
+    hold_everything held_everything (ischild);
 
-  ischild = setjmp (grouped.ch.jmp);
+    if (!held_everything)
+      {
+	if (exit_state)
+	  Sleep (INFINITE);
+	set_errno (EAGAIN);
+	return -1;
+      }
 
-  void *esp;
-  __asm__ volatile ("movl %%esp,%0": "=r" (esp));
+    ischild = !!setjmp (grouped.ch.jmp);
 
-  if (ischild)
-    {
-      res = grouped.child (esp);
-      now.dont_bother ();
-    }
-  else
-    {
+    volatile char * volatile esp;
+    __asm__ volatile ("movl %%esp,%0": "=r" (esp));
+
+
+    if (!ischild)
       res = grouped.parent (esp);
-      sig_send (NULL, __SIGNOHOLD);
-    }
+    else
+      res = grouped.child (esp);
+  }
 
   MALLOC_CHECK;
   if (ischild || res > 0)
diff --git a/winsup/cygwin/thread.cc b/winsup/cygwin/thread.cc
index fc29fc19a..936d9a55c 100644
--- a/winsup/cygwin/thread.cc
+++ b/winsup/cygwin/thread.cc
@@ -1,9 +1,7 @@
 /* thread.cc: Locking and threading module functions
 
-   Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007 Red Hat, Inc.
-
-   Originally written by Marco Fuykschot <marco@ddi.nl>
-   Substantialy enhanced by Robert Collins <rbtcollins@hotmail.com>
+   Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
+   2006, 2007 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -1599,7 +1597,11 @@ pthread_mutex::pthread_mutex (pthread_mutexattr *attr) :
   verifyable_object (PTHREAD_MUTEX_MAGIC),
   lock_counter (0),
   win32_obj_id (NULL), recursion_counter (0),
-  condwaits (0), owner (NULL), type (PTHREAD_MUTEX_ERRORCHECK),
+  condwaits (0), owner (NULL),
+#ifdef DEBUGGING
+  tid (0),
+#endif
+  type (PTHREAD_MUTEX_ERRORCHECK),
   pshared (PTHREAD_PROCESS_PRIVATE)
 {
   win32_obj_id = ::CreateSemaphore (&sec_none_nih, 0, LONG_MAX, NULL);
@@ -1680,6 +1682,9 @@ pthread_mutex::_unlock (pthread_t self)
   if (--recursion_counter == 0)
     {
       owner = NULL;
+#ifdef DEBUGGING
+      tid = 0;
+#endif
       if (InterlockedDecrement ((long *)&lock_counter))
 	// Another thread is waiting
 	::ReleaseSemaphore (win32_obj_id, 1, NULL);
@@ -1713,11 +1718,21 @@ pthread_mutex::_fixup_after_fork ()
     api_fatal ("pthread_mutex::_fixup_after_fork () doesn'tunderstand PROCESS_SHARED mutex's");
 
   if (owner == NULL)
-    /* mutex has no owner, reset to initial */
-    lock_counter = 0;
+    {
+      /* mutex has no owner, reset to initial */
+      lock_counter = 0;
+#ifdef DEBUGGING
+      tid = 0;
+#endif
+    }
   else if (lock_counter != 0)
-    /* All waiting threads are gone after a fork */
-    lock_counter = 1;
+    {
+      /* All waiting threads are gone after a fork */
+      lock_counter = 1;
+#ifdef DEBUGGING
+      tid = 0xffffffff;	/* Don't know the tid after a fork */
+#endif
+    }
 
   win32_obj_id = ::CreateSemaphore (&sec_none_nih, 0, LONG_MAX, NULL);
   if (!win32_obj_id)
diff --git a/winsup/cygwin/thread.h b/winsup/cygwin/thread.h
index a1dec1ff8..029e35745 100644
--- a/winsup/cygwin/thread.h
+++ b/winsup/cygwin/thread.h
@@ -1,9 +1,6 @@
 /* thread.h: Locking and threading module definitions
 
-   Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2007 Red Hat, Inc.
-
-   Written by Marco Fuykschot <marco@ddi.nl>
-   Major update 2001 Robert Collins <rbtcollins@hotmail.com>
+   Copyright 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2007 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -45,7 +42,8 @@ void ReleaseResourceLock (int, int, const char *)
   __attribute__ ((regparm (3)));
 }
 
-DWORD cancelable_wait (HANDLE, DWORD, const cw_cancel_action = cw_cancel_self, const enum cw_sig_wait = cw_sig_nosig)
+DWORD cancelable_wait (HANDLE, DWORD, const cw_cancel_action = cw_cancel_self,
+		       const enum cw_sig_wait = cw_sig_nosig)
   __attribute__ ((regparm (3)));
 
 class fast_mutex
@@ -298,6 +296,9 @@ public:
   unsigned int recursion_counter;
   LONG condwaits;
   pthread_t owner;
+#ifdef DEBUGGING
+  DWORD tid;		/* the thread id of the owner */
+#endif
   int type;
   int pshared;
 
@@ -328,6 +329,9 @@ public:
   {
     recursion_counter = 1;
     owner = self;
+#ifdef DEBUGGING
+    tid = GetCurrentThreadId ();
+#endif
   }
 
   int lock_recursive ()