* debug.cc (add_handle): Print handle value when collision detected.

* dtable.cc (dtable::stdio_init): Cosmetic change.
* fhandler.h (fhandler_base::create_read_state): Protect handle.
(fhandler_pipe::create_guard): Ditto.  Always mark the handle as inheritable.
(fhandler_pipe::is_slow): Return boolean value rather than numeric 1.
* pipe.cc (fhandler_pipe::fhandler_pipe): Always flag that we need fork fixup.
(fhandler_pipe::open): Don't pass security attributes to create_guard.
(fhandler_pipe::set_close_on_exec): Don't handle guard here.
(fhandler_pipe::close): Accommodate now-protected guard handle.
(fhandler_pipe::fixup_in_child): Don't proected read_state here.
(fhandler_pipe::fixup_after_exec): Close guard handle if close_on_exec.
(fhandler_pipe::fixup_after_fork): Don't bother with guard here.
(fhandler_pipe::dup): Don't set res to non-error prematurely.  Use boolean
values where appropriate.  Protect guard and read_state.
(fhandler_pipe::create): Don't call need_fork_fixup since it is now the
default.  Don't protect read_state or guard.
* pipe.cc (fhandler_base::ready_for_read): Use bool values for "avail".
* spawn.cc (spawn_guts): Set cygheap->pid_handle as inheritable when
protecting.
* select.cc (fhandler_pipe::ready_for_read): Actually get the guard mutex for
blocking reads.

6 files changed, 71 insertions, 46 deletions

diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog
index 5eef80c2c..8a24e5e75 100644
--- a/winsup/cygwin/ChangeLog
+++ b/winsup/cygwin/ChangeLog
@@ -1,3 +1,35 @@
+2006-05-21  Christopher Faylor  <cgf@timesys.com>
+
+	* debug.cc (add_handle): Print handle value when collision detected.
+	* dtable.cc (dtable::stdio_init): Cosmetic change.
+	* fhandler.h (fhandler_base::create_read_state): Protect handle.
+	(fhandler_pipe::create_guard): Ditto.  Always mark the handle as
+	inheritable.
+	(fhandler_pipe::is_slow): Return boolean value rather than numeric 1.
+	* pipe.cc (fhandler_pipe::fhandler_pipe): Always flag that we need fork
+	fixup.
+	(fhandler_pipe::open): Don't pass security attributes to create_guard.
+	(fhandler_pipe::set_close_on_exec): Don't handle guard here.
+	(fhandler_pipe::close): Accommodate now-protected guard handle.
+	(fhandler_pipe::fixup_in_child): Don't proected read_state here.
+	(fhandler_pipe::fixup_after_exec): Close guard handle if close_on_exec.
+	(fhandler_pipe::fixup_after_fork): Don't bother with guard here.
+	(fhandler_pipe::dup): Don't set res to non-error prematurely.  Use
+	boolean values where appropriate.  Protect guard and read_state.
+	(fhandler_pipe::create): Don't call need_fork_fixup since it is now the
+	default.  Don't protect read_state or guard.
+
+	* pipe.cc (fhandler_base::ready_for_read): Use bool values for "avail".
+
+	* spawn.cc (spawn_guts): Set cygheap->pid_handle as inheritable when
+	protecting.
+
+2006-05-15  Lev Bishop  <lev.bishop+cygwin@gmail.com>
+	    Christopher Faylor  <cgf@timesys.com>
+
+	* select.cc (fhandler_pipe::ready_for_read): Actually get the guard
+	mutex for blocking reads.
+
 2006-05-20  Christopher Faylor  <cgf@timesys.com>
 
 	* fhandler_tty.cc (fhandler_tty::close): Remove problematic hExeced guard.
diff --git a/winsup/cygwin/debug.cc b/winsup/cygwin/debug.cc
index be256b01b..4e22afd5d 100644
--- a/winsup/cygwin/debug.cc
+++ b/winsup/cygwin/debug.cc
@@ -152,7 +152,7 @@ add_handle (const char *func, int ln, HANDLE h, const char *name, bool inh)
   hl->pid = GetCurrentProcessId ();
   cygheap->debug.endh->next = hl;
   cygheap->debug.endh = hl;
-  debug_printf ("protecting handle '%s', inherited flag %d", hl->name, hl->inherited);
+  debug_printf ("protecting handle '%s'(%p), inherited flag %d", hl->name, hl->h, hl->inherited);
 }
 
 static void __stdcall
diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index f22be554e..85136ac61 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -154,8 +154,8 @@ dtable::stdio_init ()
     {
       /* Since this code is not invoked for forked tasks, we don't have
 	 to worry about the close-on-exec flag here.  */
-      if (!DuplicateHandle (hMainProc, out, hMainProc, &err, 0,
-			     1, DUPLICATE_SAME_ACCESS))
+      if (!DuplicateHandle (hMainProc, out, hMainProc, &err, 0, true,
+			    DUPLICATE_SAME_ACCESS))
 	{
 	  /* If that fails, do this as a fall back.  */
 	  err = out;
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index 52e41e27a..2bd4bac5a 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -212,6 +212,7 @@ class fhandler_base
   void create_read_state (LONG n)
   {
     read_state = CreateSemaphore (&sec_none_nih, 0, n, NULL);
+    ProtectHandle (read_state);
   }
 
   void signal_read_state (LONG n)
@@ -512,7 +513,11 @@ public:
   void __stdcall read (void *ptr, size_t& len) __attribute__ ((regparm (3)));
   int open (int flags, mode_t mode = 0);
   int close ();
-  void create_guard (SECURITY_ATTRIBUTES *sa) {guard = CreateMutex (sa, FALSE, NULL);}
+  void create_guard ()
+    {
+      guard = CreateMutex (&sec_none, FALSE, NULL);
+      ProtectHandleINH (guard);
+    }
   int dup (fhandler_base *child);
   int ioctl (unsigned int cmd, void *);
   void fixup_in_child ();
@@ -545,7 +550,7 @@ public:
   void set_output_handle (HANDLE h) { output_handle = h; }
   void set_use ();
   int dup (fhandler_base *child);
-  bool is_slow () {return 1;}
+  bool is_slow () {return true;}
   void close_one_end ();
 };
 
diff --git a/winsup/cygwin/pipe.cc b/winsup/cygwin/pipe.cc
index e6e9c7d0c..f913fc1ec 100644
--- a/winsup/cygwin/pipe.cc
+++ b/winsup/cygwin/pipe.cc
@@ -1,7 +1,7 @@
 /* pipe.cc: pipe for Cygwin.
 
-   Copyright 1996, 1998, 1999, 2000, 2001, 2002, 2003, 2004,
-   2005 Red Hat, Inc.
+   Copyright 1996, 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005,
+   2006 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -31,9 +31,10 @@ static unsigned pipecount;
 static const NO_COPY char pipeid_fmt[] = "stupid_pipe.%u.%u";
 
 fhandler_pipe::fhandler_pipe ()
-  : fhandler_base (), guard (NULL), broken_pipe (false), writepipe_exists(0),
+  : fhandler_base (), guard (NULL), broken_pipe (false), writepipe_exists (NULL),
     orig_pid (0), id (0)
 {
+  need_fork_fixup (true);
 }
 
 extern "C" int sscanf (const char *, const char *, ...);
@@ -99,21 +100,16 @@ fhandler_pipe::open (int flags, mode_t mode)
       goto out;
     }
   if (fh->writepipe_exists
-      && !DuplicateHandle (proc, fh->writepipe_exists,
-			   hMainProc, &nwrp_hdl,
+      && !DuplicateHandle (proc, fh->writepipe_exists, hMainProc, &nwrp_hdl,
 			   0, false, DUPLICATE_SAME_ACCESS))
     {
       __seterrno ();
       goto out;
     }
   if (fh->read_state)
-    {
-      create_read_state (2);
-      need_fork_fixup (true);
-      ProtectHandle1 (read_state, read_state);
-    }
-  if (fh->get_guard ())
-    create_guard ((flags & O_NOINHERIT) ?  &sec_none_nih : &sec_none);
+    create_read_state (2);
+  if (fh->guard)
+    create_guard ();
   init (nio_hdl, fh->get_access (), mode & O_TEXT ?: O_BINARY);
   writepipe_exists = nwrp_hdl;
   if (flags & O_NOINHERIT)
@@ -146,13 +142,12 @@ void
 fhandler_pipe::set_close_on_exec (bool val)
 {
   fhandler_base::set_close_on_exec (val);
-  if (guard)
-    set_no_inheritance (guard, val);
   if (writepipe_exists)
     set_no_inheritance (writepipe_exists, val);
 }
 
-char *fhandler_pipe::get_proc_fd_name (char *buf)
+char *
+fhandler_pipe::get_proc_fd_name (char *buf)
 {
   __small_sprintf (buf, "pipe:[%d]", get_handle ());
   return buf;
@@ -192,7 +187,7 @@ int
 fhandler_pipe::close ()
 {
   if (guard)
-    CloseHandle (guard);
+    ForceCloseHandle (guard);
   if (writepipe_exists)
     CloseHandle (writepipe_exists);
 #ifndef NEWVFORK
@@ -227,25 +222,22 @@ void
 fhandler_pipe::fixup_in_child ()
 {
   if (read_state)
-    {
-      create_read_state (2);
-      ProtectHandle (read_state);
-    }
+    create_read_state (2);
 }
 
 void
 fhandler_pipe::fixup_after_exec ()
 {
   if (!close_on_exec ())
-      fixup_in_child ();
+    fixup_in_child ();
+  else if (guard)
+    ForceCloseHandle (guard);
 }
 
 void
 fhandler_pipe::fixup_after_fork (HANDLE parent)
 {
   fhandler_base::fixup_after_fork (parent);
-  if (guard)
-    fork_fixup (parent, guard, "guard");
   if (writepipe_exists)
     fork_fixup (parent, writepipe_exists, "guard");
   fixup_in_child ();
@@ -258,17 +250,15 @@ fhandler_pipe::dup (fhandler_base *child)
   fhandler_pipe *ftp = (fhandler_pipe *) child;
   ftp->guard = ftp->writepipe_exists = ftp->read_state = NULL;
 
-  if (get_handle ())
-    {
-      res = fhandler_base::dup (child);
-      if (res)
-	goto err;
-    }
+  if (get_handle () && fhandler_base::dup (child))
+    goto err;
 
   if (guard == NULL)
     ftp->guard = NULL;
-  else if (!DuplicateHandle (hMainProc, guard, hMainProc, &ftp->guard, 0, 1,
+  else if (DuplicateHandle (hMainProc, guard, hMainProc, &ftp->guard, 0, true,
 			     DUPLICATE_SAME_ACCESS))
+    ProtectHandle1 (ftp->guard, guard);
+  else
     {
       debug_printf ("couldn't duplicate guard %p, %E", guard);
       goto err;
@@ -277,7 +267,7 @@ fhandler_pipe::dup (fhandler_base *child)
   if (writepipe_exists == NULL)
     ftp->writepipe_exists = NULL;
   else if (!DuplicateHandle (hMainProc, writepipe_exists, hMainProc,
-			     &ftp->writepipe_exists, 0, 1,
+			     &ftp->writepipe_exists, 0, false,
 			     DUPLICATE_SAME_ACCESS))
     {
       debug_printf ("couldn't duplicate writepipe_exists %p, %E", writepipe_exists);
@@ -286,9 +276,11 @@ fhandler_pipe::dup (fhandler_base *child)
 
   if (read_state == NULL)
     ftp->read_state = NULL;
-  else if (!DuplicateHandle (hMainProc, read_state, hMainProc,
+  else if (DuplicateHandle (hMainProc, read_state, hMainProc,
 			     &ftp->read_state, 0, 0,
 			     DUPLICATE_SAME_ACCESS))
+    ProtectHandle1 (ftp->read_state, read_state);
+  else
     {
       debug_printf ("couldn't duplicate read_state %p, %E", read_state);
       goto err;
@@ -299,19 +291,17 @@ fhandler_pipe::dup (fhandler_base *child)
 
 err:
   if (ftp->guard)
-    CloseHandle (ftp->guard);
+    ForceCloseHandle1 (ftp->guard, guard);
   if (ftp->writepipe_exists)
     CloseHandle (ftp->writepipe_exists);
   if (ftp->read_state)
-    CloseHandle (ftp->read_state);
+    ForceCloseHandle1 (ftp->read_state, read_state);
   goto leave;
 
 out:
   ftp->id = id;
   ftp->orig_pid = orig_pid;
-  VerifyHandle (ftp->guard);
   VerifyHandle (ftp->writepipe_exists);
-  VerifyHandle (ftp->read_state);
 
 leave:
   debug_printf ("res %d", res);
@@ -447,17 +437,15 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode, bool fif
        }
 
       fhs[0]->create_read_state (2);
-      fhs[0]->need_fork_fixup (true);
-      ProtectHandle1 (fhs[0]->read_state, read_state);
 
       res = 0;
-      fhs[0]->create_guard (sa);
+      fhs[0]->create_guard ();
       if (wincap.has_unreliable_pipes ())
 	{
 	  char buf[80];
 	  int count = pipecount++;	/* FIXME: Should this be InterlockedIncrement? */
 	  __small_sprintf (buf, pipeid_fmt, myself->pid, count);
-	  fhs[1]->writepipe_exists = CreateEvent (sa, TRUE, FALSE, buf);
+	  fhs[1]->writepipe_exists = CreateEvent (&sec_none_nih, TRUE, FALSE, buf);
 	  fhs[0]->orig_pid = myself->pid;
 	  fhs[0]->id = count;
 	}
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 94ead62f9..41f40543e 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -438,7 +438,7 @@ spawn_guts (const char * prog_arg, const char *const *argv,
 	/* already done previously */;
       else if (DuplicateHandle (hMainProc, hMainProc, hMainProc, &cygheap->pid_handle,
 				PROCESS_QUERY_INFORMATION, TRUE, 0))
-	ProtectHandle (cygheap->pid_handle);
+	ProtectHandleINH (cygheap->pid_handle);
       else
 	system_printf ("duplicate to pid_handle failed, %E");
       if (mode != _P_DETACH)