From 4aeaedf961029e38cd9c97ac1e32ba91406c9e23 Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Fri, 16 Mar 2012 20:20:29 +0000 Subject: [PATCH] * fork.cc (lock_signals): Move to sigproc.h. (lock_pthread): Ditto. (hold_everything): Ditto. (frok::parent): Call myself.prefork() just before calling CreateProcess. Call myself.postfork () on function exit. * pinfo.cc (pinfo::pending_rd_proc_pipe): Define. (pinfo::pending_wr_proc_pipe): Ditto. (_pinfo::dup_proc_pipe): Delete. (pinfo::wait): Move pipe creation into pinfo::prefork. Set pipe variables from pending_*. (_pinfo::sync_proc_pipe): Delete. (_pinfo::proc_pipe_owner): Ditto. (pinfo::prefork): Define new function. (pinfo::postfork): Ditto. (pinfo::postexec): Ditto. (_pinfo::alert_parent): Remove obsolete call to sync_proc_pipe. (_pinfo::dup_proc_pipe): Delete declaration. (_pinfo::sync_proc_pipe): Ditto. (pinfo::pending_rd_proc_pipe): Declare. (pinfo::pending_wr_proc_pipe): Ditto. (pinfo::prefork): Declare new function. (pinfo::postfork): Ditto. (pinfo::postexec): Ditto. (pinfo::wr_proc_pipe): Define new wrapper function. * sigproc.h: Include "sync.h". Move locking functions from fork to here. * spawn.cc (child_info_spawn::worker): Delete now-unneeded requirement to record orig_wr_proc_pipe. Call hold_everything prior to doing anything. Call myself.prefork() if spawning. Replace wr_proc_pipe synchronization with call to myself.postexec(). Call myself.postfork() if not execing. * sync.h: Replace #ifdef wrapper with "#pragma once". --- winsup/cygwin/ChangeLog | 35 +++++++++++++ winsup/cygwin/fork.cc | 79 +--------------------------- winsup/cygwin/pinfo.cc | 111 ++++++++++++++++++---------------------- winsup/cygwin/pinfo.h | 10 ++-- winsup/cygwin/sigproc.h | 78 ++++++++++++++++++++++++++++ winsup/cygwin/spawn.cc | 39 ++++++-------- winsup/cygwin/sync.h | 9 ++-- 7 files changed, 192 insertions(+), 169 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index d0f74cf61..ebcf00afd 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,38 @@ +2012-03-16 Christopher Faylor + + * fork.cc (lock_signals): Move to sigproc.h. + (lock_pthread): Ditto. + (hold_everything): Ditto. + (frok::parent): Call myself.prefork() just before calling + CreateProcess. Call myself.postfork () on function exit. + * pinfo.cc (pinfo::pending_rd_proc_pipe): Define. + (pinfo::pending_wr_proc_pipe): Ditto. + (_pinfo::dup_proc_pipe): Delete. + (pinfo::wait): Move pipe creation into pinfo::prefork. Set pipe + variables from pending_*. + (_pinfo::sync_proc_pipe): Delete. + (_pinfo::proc_pipe_owner): Ditto. + (pinfo::prefork): Define new function. + (pinfo::postfork): Ditto. + (pinfo::postexec): Ditto. + (_pinfo::alert_parent): Remove obsolete call to sync_proc_pipe. + (_pinfo::dup_proc_pipe): Delete declaration. + (_pinfo::sync_proc_pipe): Ditto. + (pinfo::pending_rd_proc_pipe): Declare. + (pinfo::pending_wr_proc_pipe): Ditto. + (pinfo::prefork): Declare new function. + (pinfo::postfork): Ditto. + (pinfo::postexec): Ditto. + (pinfo::wr_proc_pipe): Define new wrapper function. + * sigproc.h: Include "sync.h". Move locking functions from fork to + here. + * spawn.cc (child_info_spawn::worker): Delete now-unneeded requirement + to record orig_wr_proc_pipe. Call hold_everything prior to doing + anything. Call myself.prefork() if spawning. Replace wr_proc_pipe + synchronization with call to myself.postexec(). Call myself.postfork() + if not execing. + * sync.h: Replace #ifdef wrapper with "#pragma once". + 2012-03-13 Corinna Vinschen * hookapi.cc (hook_or_detect_cygwin): Change condition when to use diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 56ba0fc0f..2b29a1d70 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -47,83 +47,6 @@ class frok 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 -{ -public: /* DELETEME*/ - 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) { @@ -407,6 +330,7 @@ frok::parent (volatile char * volatile stack_here) while (1) { hchild = NULL; + myself.prefork (); rc = CreateProcessW (myself->progname, /* image to run */ myself->progname, /* what we send in arg0 */ &sec_none_nih, @@ -592,6 +516,7 @@ frok::parent (volatile char * volatile stack_here) /* Common cleanup code for failure cases */ cleanup: + myself.postfork (); if (fix_impersonation) cygheap->user.reimpersonate (); if (locked) diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc index d7e147ae5..c5e646455 100644 --- a/winsup/cygwin/pinfo.cc +++ b/winsup/cygwin/pinfo.cc @@ -49,6 +49,9 @@ pinfo_basic myself_initial NO_COPY; pinfo NO_COPY myself (static_cast<_pinfo *> (&myself_initial)); // Avoid myself != NULL checks +HANDLE NO_COPY pinfo::pending_rd_proc_pipe; +HANDLE NO_COPY pinfo::pending_wr_proc_pipe; + bool is_toplevel_proc; /* Setup the pinfo structure for this process. There may already be a @@ -980,66 +983,16 @@ proc_waiter (void *arg) return 0; } -#ifdef DEBUGGING -#define warn_printf api_fatal -#else -#define warn_printf system_printf -#endif -HANDLE -_pinfo::dup_proc_pipe (HANDLE hProcess, const char *func) -{ - DWORD flags = DUPLICATE_SAME_ACCESS; - HANDLE orig_wr_proc_pipe = wr_proc_pipe; - /* Can't set DUPLICATE_CLOSE_SOURCE for exec case because we could be - execing a non-cygwin process and we need to set the exit value before the - parent sees it. */ - if (this != myself || is_toplevel_proc) - flags |= DUPLICATE_CLOSE_SOURCE; - bool res = DuplicateHandle (GetCurrentProcess (), wr_proc_pipe, - hProcess, &wr_proc_pipe, 0, FALSE, flags); - if (res) - { - wr_proc_pipe_owner = dwProcessId; - sigproc_printf ("(%s) duped wr_proc_pipe %p for pid %d(%u)", func, - wr_proc_pipe, pid, dwProcessId); - } - else - { - DWORD duperr = GetLastError (); - DWORD wfsores = WaitForSingleObject (hProcess, 0); - if (wfsores != WAIT_OBJECT_0) - { - warn_printf ("(%s) process synchronization failed for pid %u/%p, " - "wr_proc_pipe %p vs. %p: DuplicateHandle winerr %d, " - "WFSO returned %u, %E", - func, pid, hProcess, wr_proc_pipe, orig_wr_proc_pipe, duperr, - wfsores); - } - wr_proc_pipe = orig_wr_proc_pipe; - } - return orig_wr_proc_pipe; -} - /* function to set up the process pipe and kick off proc_waiter */ bool pinfo::wait () { - /* If rd_proc_pipe != NULL we're in an execed process which already has - grabbed the read end of the pipe from the previous cygwin process running - with this pid. */ - if (!rd_proc_pipe) - { - /* FIXME: execed processes should be able to wait for pids that were started - by the process which execed them. */ - if (!CreatePipe (&rd_proc_pipe, &((*this)->wr_proc_pipe), &sec_none_nih, 16)) - { - system_printf ("Couldn't create pipe tracker for pid %d, %E", - (*this)->pid); - return false; - } + rd_proc_pipe = pending_rd_proc_pipe; + pending_rd_proc_pipe = NULL; - (*this)->dup_proc_pipe (hProcess, "pinfo::wait"); - } + wr_proc_pipe () = pending_wr_proc_pipe; + ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe); + pending_wr_proc_pipe = NULL; preserve (); /* Preserve the shared memory associated with the pinfo */ @@ -1059,11 +1012,48 @@ pinfo::wait () } void -_pinfo::sync_proc_pipe () +pinfo::prefork (bool detached) { - if (wr_proc_pipe && wr_proc_pipe != INVALID_HANDLE_VALUE) - while (wr_proc_pipe_owner != GetCurrentProcessId ()) - yield (); + if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE + && !SetHandleInformation (wr_proc_pipe (), HANDLE_FLAG_INHERIT, 0)) + api_fatal ("couldn't set process pipe(%p) inherit state, %E", wr_proc_pipe ()); + /* If rd_proc_pipe != NULL we're in an execed process which already has + grabbed the read end of the pipe from the previous cygwin process running + with this pid. */ + if (!detached) + { + if (!CreatePipe (&pending_rd_proc_pipe, &pending_wr_proc_pipe, + &sec_none_nih, 16)) + api_fatal ("Couldn't create pipe tracker for pid %d, %E", (*this)->pid); + + if (!SetHandleInformation (pending_wr_proc_pipe, HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT)) + api_fatal ("prefork: couldn't set process pipe(%p) inherit state, %E", + pending_wr_proc_pipe); + ProtectHandle1 (pending_rd_proc_pipe, rd_proc_pipe); + ProtectHandle1 (pending_wr_proc_pipe, wr_proc_pipe); + } +} + +void +pinfo::postfork () +{ + if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE + && !SetHandleInformation (wr_proc_pipe (), HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT)) + api_fatal ("postfork: couldn't set process pipe(%p) inherit state, %E", wr_proc_pipe ()); + if (pending_rd_proc_pipe) + ForceCloseHandle1 (pending_rd_proc_pipe, rd_proc_pipe); + if (pending_wr_proc_pipe) + ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe); +} + +void +pinfo::postexec () +{ + if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE + && !ForceCloseHandle (wr_proc_pipe ())) + api_fatal ("postexec: couldn't close wr_proc_pipe(%p), %E", wr_proc_pipe ()); } /* function to send a "signal" to the parent when something interesting happens @@ -1078,11 +1068,10 @@ _pinfo::alert_parent (char sig) FIXME: Is there a race here if we run this while another thread is attempting to exec()? */ - if (wr_proc_pipe == INVALID_HANDLE_VALUE || !myself->wr_proc_pipe || have_execed) + if (wr_proc_pipe == INVALID_HANDLE_VALUE || !myself.wr_proc_pipe () || have_execed) /* no parent */; else { - sync_proc_pipe (); if (WriteFile (wr_proc_pipe, &sig, 1, &nb, NULL)) /* all is well */; else if (GetLastError () != ERROR_BROKEN_PIPE) diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h index 551f3f29c..1e514e3c0 100644 --- a/winsup/cygwin/pinfo.h +++ b/winsup/cygwin/pinfo.h @@ -111,8 +111,6 @@ public: char *cwd (size_t &); char *cmdline (size_t &); bool set_ctty (class fhandler_termios *, int); - HANDLE dup_proc_pipe (HANDLE, const char *) __attribute__ ((regparm(3))); - void sync_proc_pipe (); bool alert_parent (char); int __stdcall kill (siginfo_t&) __attribute__ ((regparm (2))); bool __stdcall exists () __attribute__ ((regparm (1))); @@ -124,7 +122,6 @@ public: DWORD exec_dwProcessId; public: HANDLE wr_proc_pipe; - DWORD wr_proc_pipe_owner; friend class pinfo_minimal; }; @@ -150,6 +147,8 @@ class pinfo: public pinfo_minimal { bool destroy; _pinfo *procinfo; + static HANDLE pending_rd_proc_pipe; + static HANDLE pending_wr_proc_pipe; public: bool waiter_ready; class cygthread *wait_thread; @@ -205,10 +204,15 @@ public: return res; } #endif + void prefork (bool = false); + void postfork (); + void postexec (); HANDLE shared_handle () {return h;} void set_acl (); friend class _pinfo; friend class winpids; +private: + HANDLE& wr_proc_pipe() {return procinfo->wr_proc_pipe;} }; #define ISSTATE(p, f) (!!((p)->process_state & f)) diff --git a/winsup/cygwin/sigproc.h b/winsup/cygwin/sigproc.h index 2900da4cc..ffe75a8d3 100644 --- a/winsup/cygwin/sigproc.h +++ b/winsup/cygwin/sigproc.h @@ -11,6 +11,7 @@ details. */ #pragma once #include +#include "sync.h" #ifdef NSIG enum @@ -121,4 +122,81 @@ extern char myself_nowait_dummy[]; extern struct sigaction *global_sigs; +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 +{ +public: /* DELETEME*/ + 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 = false): ischild (x) {} + operator int () const {return signals;} + + ~hold_everything() + { + if (ischild) + { + pthread.dont_bother (); + process.dont_bother (); + signals.dont_bother (); + } + } + +}; + #define myself_nowait ((_pinfo *) myself_nowait_dummy) diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index c2412305a..ef0403ec0 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -308,6 +308,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, return -1; } + hold_everything for_now; /* FIXME: There is a small race here and FIXME: not thread safe! */ pthread_cleanup cleanup; @@ -335,7 +336,6 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, bool null_app_name = false; STARTUPINFOW si = {}; int looped = 0; - HANDLE orig_wr_proc_pipe = NULL; myfault efault; if (efault.faulted ()) @@ -349,10 +349,13 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, } child_info_types chtype; - if (mode != _P_OVERLAY) - chtype = _CH_SPAWN; - else + if (mode == _P_OVERLAY) chtype = _CH_EXEC; + else + { + chtype = _CH_SPAWN; + myself.prefork (); + } moreinfo = cygheap_exec_info::alloc (); @@ -770,23 +773,16 @@ loop: sigproc_printf ("new process name %W", myself->progname); /* If wr_proc_pipe doesn't exist then this process was not started by a cygwin process. So, we need to wait around until the process we've just "execed" - dies. Use our own wait facility to wait for our own pid to exit (there - is some minor special case code in proc_waiter and friends to accommodate - this). + dies. Use our own wait facility below to wait for our own pid to exit + (there is some minor special case code in proc_waiter and friends to + accommodate this). - If wr_proc_pipe exists, then it should be duplicated to the child. + If wr_proc_pipe exists, then it will be inherited by the child. If the child has exited already, that's ok. The parent will pick up - on this fact when we exit. dup_proc_pipe will close our end of the pipe. - Note that wr_proc_pipe may also be == INVALID_HANDLE_VALUE. That will make - dup_proc_pipe essentially a no-op. */ + on this fact when we exit. myself.postexec () will close our end of + the pipe. */ if (!newargv.win16_exe && myself->wr_proc_pipe) - { - if (!looped) - myself->sync_proc_pipe (); /* Make sure that we own wr_proc_pipe - just in case we've been previously - execed. */ - orig_wr_proc_pipe = myself->dup_proc_pipe (pi.hProcess, "child_info_spawn::worker"); - } + myself.postexec (); pid = myself->pid; if (!iscygwin ()) close_all_files (); @@ -851,11 +847,6 @@ loop: myself.hProcess = pi.hProcess; if (!synced) { - if (orig_wr_proc_pipe) - { - myself->wr_proc_pipe_owner = GetCurrentProcessId (); - myself->wr_proc_pipe = orig_wr_proc_pipe; - } if (!proc_retry (pi.hProcess)) { looped++; @@ -896,6 +887,8 @@ loop: } out: + if (mode != _P_OVERLAY) + myself.postfork (); this->cleanup (); if (envblock) free (envblock); diff --git a/winsup/cygwin/sync.h b/winsup/cygwin/sync.h index 2215599c8..c9c5fb595 100644 --- a/winsup/cygwin/sync.h +++ b/winsup/cygwin/sync.h @@ -1,6 +1,7 @@ /* sync.h: Header file for cygwin synchronization primitives. - Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007 Red Hat, Inc. + Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, 2011, 2012 + Red Hat, Inc. This file is part of Cygwin. @@ -8,8 +9,8 @@ This software is a copyrighted work licensed under the terms of the Cygwin license. Please consult the file "CYGWIN_LICENSE" for details. */ -#ifndef _SYNC_H -#define _SYNC_H +#pragma once + /* FIXME: Note that currently this class cannot be allocated via `new' since there are issues with malloc and fork. */ class muto @@ -62,5 +63,3 @@ public: friend class dtable; friend class fhandler_fifo; }; - -#endif /*_SYNC_H*/