* configure.in: Remove NEWVFORK default.

* configure: Regenerate.
* dcrt0.cc: Conditionalize vfork stuff throughout.
* dtable.cc: Ditto.
* perthread.h: Ditto.
* pipe.cc (fhandler_pipe::close): Ditto.
* spawn.cc (spawnve): Ditto.
* syscalls.cc (setsid): Ditto.
* exceptions.cc (sigpacket::process): Use macro to refer to vfork pid.
* debug.cc (verify_handle): Define new function.
* debug.h (VerifyHandle): Define new macro.
(verify_handle): Declare new function
* fhandler.cc (fhandler_base::dup): Verify that dup'ed handle is not supposed
to be in use.
(fhandler_base::set_inheritance): Ditto.
(fhandler_base::fork_fixup): Ditto.
* fhandler_socket.cc (fhandler_socket::dup): Ditto.
* fhandler_tty.cc (fhandler_tty_slave::open): Ditto.
* net.cc (set_socket_inheritance): Ditto.
* pinfo.cc (pinfo_fixup_after_exec): Ditto.
* sigproc.cc (proc_subproc): Ditto.
(sig_send): Ditto.
* spawn.cc (spawn_guts): Ditto.
* thread.cc (pthread::init_mainthread): Ditto.
* pipe.cc (fhandler_pipe::close): Close read_state with ForceCloseHandle since
it was protected.
(fhandler_pipe::fixup_after_exec): Protect read_state handle.
(fhandler_pipe::dup): Correctly close open handles on error condition.  Verify
that dup'ed handle is not supposed to be in use.
(fhandler_pipe::create): Protect read_state.

winsup/cygwin/ChangeLog

@@ -1,3 +1,37 @@
+2004-01-23  Christopher Faylor  <cgf@redhat.com>
+
+	* configure.in: Remove NEWVFORK default.
+	* configure: Regenerate.
+	* dcrt0.cc: Conditionalize vfork stuff throughout.
+	* dtable.cc: Ditto.
+	* perthread.h: Ditto.
+	* pipe.cc (fhandler_pipe::close): Ditto.
+	* spawn.cc (spawnve): Ditto.
+	* syscalls.cc (setsid): Ditto.
+	* exceptions.cc (sigpacket::process): Use macro to refer to vfork pid.
+
+	* debug.cc (verify_handle): Define new function.
+	* debug.h (VerifyHandle): Define new macro.
+	(verify_handle): Declare new function
+	* fhandler.cc (fhandler_base::dup): Verify that dup'ed handle is not
+	supposed to be in use.
+	(fhandler_base::set_inheritance): Ditto.
+	(fhandler_base::fork_fixup): Ditto.
+	* fhandler_socket.cc (fhandler_socket::dup): Ditto.
+	* fhandler_tty.cc (fhandler_tty_slave::open): Ditto.
+	* net.cc (set_socket_inheritance): Ditto.
+	* pinfo.cc (pinfo_fixup_after_exec): Ditto.
+	* sigproc.cc (proc_subproc): Ditto.
+	(sig_send): Ditto.
+	* spawn.cc (spawn_guts): Ditto.
+	* thread.cc (pthread::init_mainthread): Ditto.
+	* pipe.cc (fhandler_pipe::close): Close read_state with
+	ForceCloseHandle since it was protected.
+	(fhandler_pipe::fixup_after_exec): Protect read_state handle.
+	(fhandler_pipe::dup): Correctly close open handles on error condition.
+	Verify that dup'ed handle is not supposed to be in use.
+	(fhandler_pipe::create): Protect read_state.
+
 2004-01-23  Christopher Faylor  <cgf@redhat.com>
 
 	* exceptions.cc (sig_handle_tty_stop): Fix boneheaded mistake by using

winsup/cygwin/configure

@@ -1924,7 +1924,7 @@ fi
 
 case "$vfork" in
 no)	 ;;
-yes|*)	 cat >> confdefs.h <<EOF
+yes)	 cat >> confdefs.h <<EOF
 #define NEWVFORK 1
 EOF
  ;;

winsup/cygwin/configure.in

@@ -151,7 +151,7 @@ vfork="${enableval}"
 
 case "$vfork" in
 no)	 ;;
-yes|*)	 AC_DEFINE_UNQUOTED(NEWVFORK) ;;
+yes)	 AC_DEFINE_UNQUOTED(NEWVFORK) ;;
 esac
 
 dnl The only time we might want to transform the install names

winsup/cygwin/cygheap.cc

@@ -178,6 +178,9 @@ init_cygheap::close_ctty ()
   debug_printf ("closing cygheap->ctty %p", cygheap->ctty);
   int usecount = cygheap->ctty->usecount;
   cygheap->ctty->close ();
+#ifndef NEWVFORK
+  cygheap->ctty = NULL;
+#else  // FIXME: This code ain't right
   if (cygheap->ctty_on_hold == cygheap->ctty)
     cygheap->ctty_on_hold = NULL;
   if (usecount == 1)
@@ -185,6 +188,7 @@ init_cygheap::close_ctty ()
       cygheap->ctty = NULL;
       debug_printf ("setting cygheap->ctty to NULL");
     }
+#endif
 }
 
 #define pagetrunc(x) ((void *) (((DWORD) (x)) & ~(4096 - 1)))

winsup/cygwin/cygheap.h

@@ -263,7 +263,9 @@ struct init_cygheap
   struct sigaction *sigs;
 
   fhandler_tty_slave *ctty;	/* Current tty */
+#ifdef NEWVFORK
   fhandler_tty_slave *ctty_on_hold;
+#endif
   struct _threadinfo **threadlist;
   size_t sthreads;
   int open_fhs;

winsup/cygwin/dcrt0.cc

@@ -45,10 +45,14 @@ HANDLE NO_COPY hMainProc = (HANDLE) -1;
 HANDLE NO_COPY hMainThread;
 
 per_thread_waitq NO_COPY waitq_storage;
+#ifdef NEWVFORK
 per_thread_vfork NO_COPY vfork_storage;
+#endif
 
 per_thread NO_COPY *threadstuff[] = {&waitq_storage,
+#ifdef NEWVFORK
 				     &vfork_storage,
+#endif
 				     NULL};
 
 bool display_title;
@@ -59,7 +63,9 @@ codepage_type current_codepage = ansi_cp;
 int __argc_safe;
 int _declspec(dllexport) __argc;
 char _declspec(dllexport) **__argv;
+#ifdef NEWVFORK
 vfork_save NO_COPY *main_vfork;
+#endif
 
 static int NO_COPY envc;
 char NO_COPY **envp;
@@ -764,9 +770,10 @@ dll_crt0_1 (char *)
      Need to do this before any helper threads start. */
   debug_init ();
 
+#ifdef NEWVFORK
   cygheap->fdtab.vfork_child_fixup ();
-
   main_vfork = vfork_storage.create ();
+#endif
 
   cygbench ("pre-forkee");
   if (user_data->forkee)
@@ -993,12 +1000,14 @@ do_exit (int status)
 {
   syscall_printf ("do_exit (%d), exit_state %d", status, exit_state);
 
+#ifdef NEWVFORK
   vfork_save *vf = vfork_storage.val ();
   if (vf != NULL && vf->pid < 0)
     {
       exit_state = ES_NOT_EXITING;
       vf->restore_exit (status);
     }
+#endif
 
   EnterCriticalSection (&exit_lock);
   muto::set_exiting_thread ();

winsup/cygwin/debug.cc

@@ -78,7 +78,18 @@ out:
   return hl;
 }
 
-#ifdef DEBUGGING_AND_FDS_PROTECTED
+void
+verify_handle (const char *func, int ln, HANDLE h)
+{
+  handle_list *hl = find_handle (h);
+  if (!hl)
+    return;
+  system_printf ("%s:%d - multiple attempts to add handle %p", func, ln, h);
+
+  system_printf (" previously allocated by %s:%d(%s<%p>) winpid %d",
+		 hl->func, hl->ln, hl->name, hl->h, hl->pid);
+}
+
 void
 setclexec (HANDLE oh, HANDLE nh, bool not_inheriting)
 {
@@ -90,7 +101,6 @@ setclexec (HANDLE oh, HANDLE nh, bool not_inheriting)
       hl->h = nh;
     }
 }
-#endif
 
 /* Create a new handle record */
 static handle_list * __stdcall

winsup/cygwin/debug.h

@@ -37,6 +37,7 @@ details. */
 # define debug_init() do {} while (0)
 # define setclexec(h, nh, b) do {} while (0)
 # define debug_fixup_after_fork_exec() do {} while (0)
+# define VerifyHandle(h) do {} while (0)
 
 #else
 
@@ -59,10 +60,13 @@ details. */
 # define ProtectHandleINH(h) add_handle (__PRETTY_FUNCTION__, __LINE__, (h), #h, 1)
 # define ProtectHandle1INH(h, n) add_handle (__PRETTY_FUNCTION__, __LINE__, (h), #n, 1)
 # define ProtectHandle2INH(h, n) add_handle (__PRETTY_FUNCTION__, __LINE__, (h), n, 1)
+# define VerifyHandle(h) verify_handle (__PRETTY_FUNCTION__, __LINE__, (h))
 
 void debug_init ();
 void __stdcall add_handle (const char *, int, HANDLE, const char *, bool = false)
   __attribute__ ((regparm (3)));
+void __stdcall verify_handle (const char *, int, HANDLE)
+  __attribute__ ((regparm (3)));
 bool __stdcall close_handle (const char *, int, HANDLE, const char *, bool)
   __attribute__ ((regparm (3)));
 void __stdcall cygbench (const char *s) __attribute__ ((regparm (1)));

winsup/cygwin/dtable.cc

@@ -705,6 +705,7 @@ dtable::fixup_after_fork (HANDLE parent)
       }
 }
 
+#ifdef NEWVFORK
 int
 dtable::vfork_child_dup ()
 {
@@ -803,6 +804,7 @@ dtable::vfork_child_fixup ()
 
   return;
 }
+#endif /*NEWVFORK*/
 
 #define DEVICE_PREFIX "\\device\\"
 #define DEVICE_PREFIX_LEN sizeof (DEVICE_PREFIX) - 1

winsup/cygwin/dtable.h

@@ -22,7 +22,9 @@ class dtable
 {
   muto *lock_cs;
   fhandler_base **fds;
+#ifdef NEWVFORK
   fhandler_base **fds_on_hold;
+#endif
   fhandler_base **archetypes;
   unsigned narchetypes;
   unsigned farchetype;
@@ -74,7 +76,9 @@ public:
   void stdio_init ();
   void get_debugger_info ();
   void set_file_pointers_for_exec ();
+#ifdef NEWVFORK
   bool in_vfork_cleanup () {return fds_on_hold == fds;}
+#endif
   fhandler_fifo *find_fifo (ATOM);
   fhandler_base *find_archetype (device& dev);
   fhandler_base **add_archetype ();

winsup/cygwin/exceptions.cc

@@ -957,7 +957,7 @@ sigpacket::process ()
   bool special_case;
   bool insigwait_mask;
   insigwait_mask = masked = false;
-  if (special_case = (main_vfork->pid || ISSTATE (myself, PID_STOPPED)))
+  if (special_case = (VFORKPID || ISSTATE (myself, PID_STOPPED)))
     /* nothing to do */;
   else if (tls && sigismember (&tls->sigwait_mask, si.si_signo))
     insigwait_mask = true;

winsup/cygwin/fhandler.cc

@@ -1044,6 +1044,7 @@ fhandler_base::dup (fhandler_base *child)
 	  return -1;
 	}
 
+      VerifyHandle (nh);
       child->set_io_handle (nh);
     }
   return 0;
@@ -1209,17 +1210,17 @@ fhandler_dev_null::dump (void)
 void
 fhandler_base::set_inheritance (HANDLE &h, int not_inheriting)
 {
-#ifdef DEBUGGING_AND_FDS_PROTECTED
   HANDLE oh = h;
-#endif
   /* Note that we could use SetHandleInformation here but it is not available
      on all platforms.  Test cases seem to indicate that using DuplicateHandle
      in this fashion does not actually close the original handle, which is
      what we want.  If this changes in the future, we may be forced to use
      SetHandleInformation on newer OS's */
-  if (!DuplicateHandle (hMainProc, h, hMainProc, &h, 0, !not_inheriting,
+  if (!DuplicateHandle (hMainProc, oh, hMainProc, &h, 0, !not_inheriting,
 			     DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE))
     debug_printf ("DuplicateHandle failed, %E");
+  if (oh != h)
+    VerifyHandle (h);
 #ifdef DEBUGGING_AND_FDS_PROTECTED
   if (h)
     setclexec (oh, h, not_inheriting);
@@ -1229,17 +1230,14 @@ fhandler_base::set_inheritance (HANDLE &h, int not_inheriting)
 void
 fhandler_base::fork_fixup (HANDLE parent, HANDLE &h, const char *name)
 {
+  HANDLE oh = h;
   if (/* !is_socket () && */ !get_close_on_exec ())
     debug_printf ("handle %p already opened", h);
   else if (!DuplicateHandle (parent, h, hMainProc, &h, 0, !get_close_on_exec (),
 			     DUPLICATE_SAME_ACCESS))
     system_printf ("%s - %E, handle %s<%p>", get_name (), name, h);
-#ifdef DEBUGGING_AND_FDS_PROTECTED
+  else if (oh != h)
-  else if (get_close_on_exec ())
+    VerifyHandle (h);
-    ProtectHandle (h);	/* would have to be fancier than this */
-  else
-    /* ProtectHandleINH (h) */;	/* Should already be protected */
-#endif
 }
 
 void

winsup/cygwin/fhandler_socket.cc

@@ -363,6 +363,7 @@ fhandler_socket::dup (fhandler_base *child)
       __seterrno ();
       return -1;
     }
+  VerifyHandle (nh);
   fhs->set_io_handle (nh);
   return 0;
 }

winsup/cygwin/fhandler_tty.cc

@@ -553,6 +553,7 @@ fhandler_tty_slave::open (int flags, mode_t)
 	  return 0;
 	}
 
+      VerifyHandle (from_master_local);
       if (!DuplicateHandle (tty_owner, get_ttyp ()->to_master,
 			  hMainProc, &to_master_local, 0, TRUE,
 			  DUPLICATE_SAME_ACCESS))
@@ -561,6 +562,7 @@ fhandler_tty_slave::open (int flags, mode_t)
 	  __seterrno ();
 	  return 0;
 	}
+      VerifyHandle (to_master_local);
       CloseHandle (tty_owner);
     }
 

winsup/cygwin/net.cc

@@ -130,6 +130,7 @@ set_socket_inheritance (SOCKET sock)
     system_printf ("DuplicateHandle failed %E");
   else
     debug_printf ("DuplicateHandle succeeded osock %p, sock %p", osock, sock);
+  VerifyHandle ((HANDLE) sock);
   return sock;
 }
 

winsup/cygwin/perthread.h

@@ -43,8 +43,14 @@ public:
   size_t size () {return sizeof (waitq);}
 };
 
-#if defined (NEED_VFORK)
+#ifdef NEED_VFORK
 #include "cygtls.h"
+#endif
+
+#ifndef NEWVFORK
+#define VFORKPID 0
+#else
+#if defined (NEED_VFORK)
 class vfork_save
 {
   jmp_buf j;
@@ -82,7 +88,9 @@ public:
 };
 extern per_thread_vfork vfork_storage;
 extern vfork_save *main_vfork;
+#define VFORKPID main_vfork->pid
 #endif
+#endif /*NEWVFORK*/
 
 extern per_thread_waitq waitq_storage;
 

winsup/cygwin/pinfo.cc

@@ -49,6 +49,7 @@ pinfo_fixup_after_fork ()
       system_printf ("couldn't save current process handle %p, %E", hMainProc);
       hexec_proc = NULL;
     }
+  VerifyHandle (hexec_proc);
 }
 
 /* Initialize the process table.

winsup/cygwin/pipe.cc

@@ -89,11 +89,15 @@ fhandler_pipe::close ()
     CloseHandle (guard);
   if (writepipe_exists)
     CloseHandle (writepipe_exists);
+#ifndef NEWVFORK
+  if (read_state)
+#else
   // FIXME is this vfork_cleanup test right?  Is it responsible for some of
   // the strange pipe behavior that has been reported in the cygwin mailing
   // list?
   if (read_state && !cygheap->fdtab.in_vfork_cleanup ())
-    CloseHandle (read_state);
+#endif
+    ForceCloseHandle (read_state);
   if (get_handle ())
     {
       CloseHandle (get_handle ());
@@ -122,7 +126,10 @@ void
 fhandler_pipe::fixup_after_exec (HANDLE parent)
 {
   if (read_state)
-    read_state = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
+    {
+      read_state = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
+      ProtectHandle (read_state);
+    }
 }
 
 void
@@ -139,15 +146,17 @@ fhandler_pipe::fixup_after_fork (HANDLE parent)
 int
 fhandler_pipe::dup (fhandler_base *child)
 {
+  int res = -1;
+  fhandler_pipe *ftp = (fhandler_pipe *) child;
+  ftp->guard = ftp->writepipe_exists = ftp->read_state = NULL;
+
   if (get_handle ())
     {
-      int res = fhandler_base::dup (child);
+      res = fhandler_base::dup (child);
       if (res)
-	return res;
+	goto err;
     }
 
-  fhandler_pipe *ftp = (fhandler_pipe *) child;
-
   /* FIXME: This leaks handles in the failing condition */
   if (guard == NULL)
     ftp->guard = NULL;
@@ -155,7 +164,7 @@ fhandler_pipe::dup (fhandler_base *child)
 			     DUPLICATE_SAME_ACCESS))
     {
       debug_printf ("couldn't duplicate guard %p, %E", guard);
-      return -1;
+      goto err;
     }
 
   if (writepipe_exists == NULL)
@@ -165,7 +174,7 @@ fhandler_pipe::dup (fhandler_base *child)
 			     DUPLICATE_SAME_ACCESS))
     {
       debug_printf ("couldn't duplicate writepipe_exists %p, %E", writepipe_exists);
-      return -1;
+      goto err;
     }
 
   if (read_state == NULL)
@@ -175,12 +184,31 @@ fhandler_pipe::dup (fhandler_base *child)
 			     DUPLICATE_SAME_ACCESS))
     {
       debug_printf ("couldn't duplicate read_state %p, %E", writepipe_exists);
-      return -1;
+      goto err;
     }
 
+  res = 0;
+  goto out;
+
+err:
+  if (!ftp->guard)
+    CloseHandle (ftp->guard);
+  if (!ftp->writepipe_exists)
+    CloseHandle (ftp->writepipe_exists);
+  if (!ftp->read_state)
+    CloseHandle (ftp->read_state);
+  goto leave;
+
+out:
   ftp->id = id;
   ftp->orig_pid = orig_pid;
-  return 0;
+  VerifyHandle (ftp->guard);
+  VerifyHandle (ftp->writepipe_exists);
+  VerifyHandle (ftp->read_state);
+
+leave:
+  debug_printf ("res %d", res);
+  return res;
 }
 
 int
@@ -208,6 +236,7 @@ fhandler_pipe::create (fhandler_pipe *fhs[2], unsigned psize, int mode, bool fif
 
       fhs[0]->read_state = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
       fhs[0]->set_need_fork_fixup ();
+      ProtectHandle1 (fhs[0]->read_state, read_state);
 
       res = 0;
       fhs[0]->create_guard (sa);

winsup/cygwin/sigproc.cc

@@ -320,6 +320,7 @@ proc_subproc (DWORD what, DWORD val)
       if (!DuplicateHandle (hMainProc, hMainProc, vchild->hProcess, &vchild->ppid_handle,
 			    SYNCHRONIZE | PROCESS_DUP_HANDLE, TRUE, 0))
 	system_printf ("Couldn't duplicate my handle<%p> for pid %d, %E", hMainProc, vchild->pid);
+      VerifyHandle (vchild->ppid_handle);
       vchild->ppid = myself->pid;
       vchild->uid = myself->uid;
       vchild->gid = myself->gid;
@@ -716,17 +717,19 @@ sig_send (_pinfo *p, siginfo_t& si, _threadinfo *tls)
 	  __seterrno ();
 	  goto out;
 	}
+      VerifyHandle (hp);
       for (int i = 0; !p->sendsig && i < 10000; i++)
 	low_priority_sleep (0);
       if (!DuplicateHandle (hp, p->sendsig, hMainProc, &sendsig, false, 0,
 			    DUPLICATE_SAME_ACCESS) || !sendsig)
 	{
+	  CloseHandle (hp);
 	  sigproc_printf ("DuplicateHandle failed, %E");
 	  __seterrno ();
 	  goto out;
 	}
       CloseHandle (hp);
-      pack.wakeup = NULL;
+      VerifyHandle (sendsig);
     }
 
   sigproc_printf ("sendsig %p, pid %d, signal %d, its_me %d", sendsig, p->pid, si.si_signo, its_me);

winsup/cygwin/spawn.cc

@@ -404,6 +404,7 @@ spawn_guts (const char * prog_arg, const char *const *argv,
        return -1;
      }
 
+  VerifyHandle (ciresrv.parent);
   ciresrv.moreinfo = (cygheap_exec_info *) ccalloc (HEAP_1_EXEC, 1, sizeof (cygheap_exec_info));
   ciresrv.moreinfo->old_title = NULL;
 
@@ -612,6 +613,8 @@ spawn_guts (const char * prog_arg, const char *const *argv,
 			&ciresrv.moreinfo->myself_pinfo, 0,
 			TRUE, DUPLICATE_SAME_ACCESS))
     ciresrv.moreinfo->myself_pinfo = NULL;
+  else
+    VerifyHandle (ciresrv.moreinfo->myself_pinfo);
 
  skip_arg_parsing:
   PROCESS_INFORMATION pi = {NULL, 0, 0, 0};
@@ -901,6 +904,7 @@ spawn_guts (const char * prog_arg, const char *const *argv,
 				    0, FALSE, DUPLICATE_SAME_ACCESS);
 	  sigproc_printf ("%d = DuplicateHandle, oldh %p, newh %p",
 			  rc, oldh, myself->hProcess);
+	  VerifyHandle (myself->hProcess);
 	  if (!rc && my_parent_is_alive ())
 	    {
 	      system_printf ("Reparent failed, parent handle %p, %E", h);
@@ -958,12 +962,14 @@ spawnve (int mode, const char *path, const char *const *argv,
 	 const char *const *envp)
 {
   int ret;
+#ifdef NEWVFORK
   vfork_save *vf = vfork_storage.val ();
 
   if (vf != NULL && (vf->pid < 0) && mode == _P_OVERLAY)
     mode = _P_NOWAIT;
   else
     vf = NULL;
+#endif
 
   syscall_printf ("spawnve (%s, %s, %x)", path, argv[0], envp);
 
@@ -984,6 +990,7 @@ spawnve (int mode, const char *path, const char *const *argv,
     case _P_SYSTEM:
       subproc_init ();
       ret = spawn_guts (path, argv, envp, mode);
+#ifdef NEWVFORK
       if (vf)
 	{
 	  if (ret > 0)
@@ -992,6 +999,7 @@ spawnve (int mode, const char *path, const char *const *argv,
 	      vf->restore_pid (ret);
 	    }
 	}
+#endif
       break;
     default:
       set_errno (EINVAL);

winsup/cygwin/syscalls.cc

@@ -300,6 +300,7 @@ getppid ()
 extern "C" pid_t
 setsid (void)
 {
+#ifdef NEWVFORK
   vfork_save *vf = vfork_storage.val ();
   /* This is a horrible, horrible kludge */
   if (vf && vf->pid < 0)
@@ -312,6 +313,7 @@ setsid (void)
 	}
       /* assuming that fork was successful */
     }
+#endif
 
   if (myself->pgid == myself->pid)
     syscall_printf ("hmm.  pgid %d pid %d", myself->pgid, myself->pid);

winsup/cygwin/thread.cc

@@ -144,6 +144,7 @@ pthread::init_mainthread ()
     api_fatal ("failed to create mainthread handle");
   if (!thread->create_cancel_event ())
     api_fatal ("couldn't create cancel event for main thread");
+  VerifyHandle (thread->win32_obj_id);
   thread->postcreate ();
 }