From 1fb6667f1ca346ab7f845b1adcb146a7d6e243fc Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Tue, 20 Mar 2012 15:07:30 +0000 Subject: [PATCH] * child_info.h (CURR_CHILD_INFO_MAGIC): Reset. (child_info::rd_proc_pipe): Declare new field. (child_info::wr_proc_pipe): Ditto. (child_info::prefork): Declare new function, derived from previous pinfo version. * dcrt0.cc (child_info_fork::handle_fork): Close previous wr_proc_pipe when appropriate and assign new one from passed-in child_info block. (child_info_spawn::handle_spawn): Assign our wr_proc_pipe handle from passed-in child_info block. * fork.cc (child_info::prefork): Define new function. (frok::child): Clear rd_proc_pipe and wr_proc_pipe so they will not be closed by the child_info destructor. (frok::parent): Use child_info prefork handling, outside of retry loop. Set rd_proc_pipe in child's pinfo after successful CreateProcess. Eliminate postfork call. * globals.cc (my_wr_proc_pipe): Define/declare new variable. * pinfo.cc (pinfo::pending_rd_proc_pipe): Delete. (pinfo::pending_wr_proc_pipe): Ditto. (pinfo::prefork): Ditto. (pinfo::postfork): Ditto. (pinfo::postexec): Ditto. (pinfo::wait): Assume that rd_proc_pipe is set up correctly prior to call. (_pinfo::alert_parent): Replace "wr_proc_pipe" with "my_wr_proc_pipe". * pinfo.h (_pinfo::_wr_proc_pipe): Delete declaration. (_pinfo::set_rd_proc_pipe): Define new function. (pinfo::pending_rd_proc_pipe): Delete declaration. (pinfo::pending_wr_proc_pipe): Ditto. (pinfo::prefork): Ditto. (pinfo::postfork): Ditto. (pinfo::postexec): Ditto. (pinfo::wr_proc_pipe): Ditto. * sigproc.cc (child_info::child_info): Clear rd_proc_pipe and wr_proc_pipe. (child_info::cleanup): Close rd_proc_pipe and wr_proc_pipe if necessary. (child_info_fork::child_info_fork): Set forker_finished to NULL by default. (child_info_spawn::child_info_spawn): Use my_wr_proc_pipe rather than myself->wr_proc_pipe. (child_info::sync): Ditto. (child_info_spawn::cleanup): Call child_info::cleanup. * spawn.cc (child_info_spawn::worker): Remove call to myself.prefork(). Set wr_proc_pipe when execing or set up new rd_proc_pipe/wr_proc_pipe via child_info::prefork when spawning. Remove call to pinfo::postexec. Set rd_proc_pipe in child pinfo when spawning. Use my_wr_proc_pipe rather than myself->wr_proc_pipe. Remove call to postfork. --- winsup/cygwin/ChangeLog | 50 +++++++++++++++++++++++++ winsup/cygwin/child_info.h | 6 ++- winsup/cygwin/dcrt0.cc | 18 +++++++++ winsup/cygwin/fork.cc | 27 ++++++++++++- winsup/cygwin/globals.cc | 1 + winsup/cygwin/pinfo.cc | 77 ++------------------------------------ winsup/cygwin/pinfo.h | 9 +---- winsup/cygwin/sigproc.cc | 41 ++++++++++++++++---- winsup/cygwin/spawn.cc | 30 ++++++--------- 9 files changed, 148 insertions(+), 111 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 96592d32a..7ce112a22 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,53 @@ +2012-03-20 Christopher Faylor + + * child_info.h (CURR_CHILD_INFO_MAGIC): Reset. + (child_info::rd_proc_pipe): Declare new field. + (child_info::wr_proc_pipe): Ditto. + (child_info::prefork): Declare new function, derived from previous + pinfo version. + * dcrt0.cc (child_info_fork::handle_fork): Close previous wr_proc_pipe + when appropriate and assign new one from passed-in child_info block. + (child_info_spawn::handle_spawn): Assign our wr_proc_pipe handle from + passed-in child_info block. + * fork.cc (child_info::prefork): Define new function. + (frok::child): Clear rd_proc_pipe and wr_proc_pipe so they will not be + closed by the child_info destructor. + (frok::parent): Use child_info prefork handling, outside of retry loop. + Set rd_proc_pipe in child's pinfo after successful CreateProcess. + Eliminate postfork call. + * globals.cc (my_wr_proc_pipe): Define/declare new variable. + * pinfo.cc (pinfo::pending_rd_proc_pipe): Delete. + (pinfo::pending_wr_proc_pipe): Ditto. + (pinfo::prefork): Ditto. + (pinfo::postfork): Ditto. + (pinfo::postexec): Ditto. + (pinfo::wait): Assume that rd_proc_pipe is set up correctly prior to + call. + (_pinfo::alert_parent): Replace "wr_proc_pipe" with "my_wr_proc_pipe". + * pinfo.h (_pinfo::_wr_proc_pipe): Delete declaration. + (_pinfo::set_rd_proc_pipe): Define new function. + (pinfo::pending_rd_proc_pipe): Delete declaration. + (pinfo::pending_wr_proc_pipe): Ditto. + (pinfo::prefork): Ditto. + (pinfo::postfork): Ditto. + (pinfo::postexec): Ditto. + (pinfo::wr_proc_pipe): Ditto. + * sigproc.cc (child_info::child_info): Clear rd_proc_pipe and + wr_proc_pipe. + (child_info::cleanup): Close rd_proc_pipe and + wr_proc_pipe if necessary. + (child_info_fork::child_info_fork): Set forker_finished to NULL by + default. + (child_info_spawn::child_info_spawn): Use my_wr_proc_pipe rather than + myself->wr_proc_pipe. + (child_info::sync): Ditto. + (child_info_spawn::cleanup): Call child_info::cleanup. + * spawn.cc (child_info_spawn::worker): Remove call to myself.prefork(). + Set wr_proc_pipe when execing or set up new rd_proc_pipe/wr_proc_pipe + via child_info::prefork when spawning. Remove call to pinfo::postexec. + Set rd_proc_pipe in child pinfo when spawning. Use my_wr_proc_pipe + rather than myself->wr_proc_pipe. Remove call to postfork. + 2012-03-19 Christopher Faylor * pinfo.cc (pinfo_init): Cosmetic change: unset "destroy" for myself. diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index 1dfa51bf5..8d1d4e906 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -35,7 +35,7 @@ enum child_status #define EXEC_MAGIC_SIZE sizeof(child_info) /* Change this value if you get a message indicating that it is out-of-sync. */ -#define CURR_CHILD_INFO_MAGIC 0x76041b78U +#define CURR_CHILD_INFO_MAGIC 0xa49e665eU #define NPROCS 256 @@ -61,6 +61,8 @@ public: void *cygheap_max; unsigned char flag; int retry; // number of times we've tried to start child process + HANDLE rd_proc_pipe; + HANDLE wr_proc_pipe; HANDLE subproc_ready; // used for synchronization with parent HANDLE user_h; HANDLE parent; @@ -78,6 +80,8 @@ public: bool isstraced () const {return !!(flag & _CI_STRACED);} bool iscygwin () const {return !!(flag & _CI_ISCYGWIN);} bool saw_ctrl_c () const {return !!(flag & _CI_SAW_CTRL_C);} + void prefork (bool = false); + void cleanup (); }; class mount_info; diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc index e7a35f219..d35ca6bae 100644 --- a/winsup/cygwin/dcrt0.cc +++ b/winsup/cygwin/dcrt0.cc @@ -598,6 +598,17 @@ child_info_fork::handle_fork () "user heap", cygheap->user_heap.base, cygheap->user_heap.ptr, NULL); + /* If my_wr_proc_pipe != NULL then it's a leftover handle from a previously + forked process. Close it now or suffer confusion with the parent of our + parent. */ + if (my_wr_proc_pipe) + ForceCloseHandle1 (my_wr_proc_pipe, wr_proc_pipe); + + /* Setup our write end of the process pipe. Clear the one in the structure. + The destructor should never be called for this but, it can't hurt to be + safe. */ + my_wr_proc_pipe = wr_proc_pipe; + rd_proc_pipe = wr_proc_pipe = NULL; /* Do the relocations here. These will actually likely be overwritten by the below child_copy but we do them here in case there is a read-only section which does not get copied by fork. */ @@ -626,6 +637,13 @@ child_info_spawn::handle_spawn () GetCurrentProcess (), &h, 0, FALSE, DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) h = NULL; + + /* Setup our write end of the process pipe. Clear the one in the structure. + The destructor should never be called for this but, it can't hurt to be + safe. */ + my_wr_proc_pipe = wr_proc_pipe; + rd_proc_pipe = wr_proc_pipe = NULL; + myself.thisproc (h); __argc = moreinfo->argc; __argv = moreinfo->argv; diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index 2b29a1d70..d014cc992 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -109,6 +109,25 @@ frok::error (const char *fmt, ...) return true; } +/* Set up a pipe which will track the life of a "pid" through + even after we've exec'ed. */ +void +child_info::prefork (bool detached) +{ + if (!detached) + { + if (!CreatePipe (&rd_proc_pipe, &wr_proc_pipe, &sec_none_nih, 16)) + api_fatal ("prefork: couldn't create pipe process tracker%E"); + + if (!SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT, + HANDLE_FLAG_INHERIT)) + api_fatal ("prefork: couldn't set process pipe(%p) inherit state, %E", + wr_proc_pipe); + ProtectHandle1 (rd_proc_pipe, rd_proc_pipe); + ProtectHandle1 (wr_proc_pipe, wr_proc_pipe); + } +} + int __stdcall frok::child (volatile char * volatile here) { @@ -194,6 +213,10 @@ frok::child (volatile char * volatile here) ld_preload (); fixup_hooks_after_fork (); _my_tls.fixup_after_fork (); + /* Clear this or the destructor will close them. In the case of + rd_proc_pipe that would be an invalid handle. In the case of + wr_proc_pipe it would be == my_wr_proc_pipe. Both would be bad. */ + ch.rd_proc_pipe = ch.wr_proc_pipe = NULL; cygwin_finished_initializing = true; return 0; } @@ -326,11 +349,11 @@ frok::parent (volatile char * volatile stack_here) cygheap->user.deimpersonate (); fix_impersonation = true; ch.refresh_cygheap (); + ch.prefork (); /* set up process tracking pipes. */ while (1) { hchild = NULL; - myself.prefork (); rc = CreateProcessW (myself->progname, /* image to run */ myself->progname, /* what we send in arg0 */ &sec_none_nih, @@ -403,6 +426,7 @@ frok::parent (volatile char * volatile stack_here) /* Fill in fields in the child's process table entry. */ child->dwProcessId = pi.dwProcessId; child.hProcess = hchild; + child.set_rd_proc_pipe (ch.rd_proc_pipe); /* Hopefully, this will succeed. The alternative to doing things this way is to reserve space prior to calling CreateProcess and then fill @@ -516,7 +540,6 @@ 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/globals.cc b/winsup/cygwin/globals.cc index 395291dc8..5a0e096d5 100644 --- a/winsup/cygwin/globals.cc +++ b/winsup/cygwin/globals.cc @@ -21,6 +21,7 @@ details. */ HANDLE NO_COPY hMainThread; HANDLE NO_COPY hProcToken; HANDLE NO_COPY hProcImpToken; +HANDLE my_wr_proc_pipe; HMODULE NO_COPY cygwin_hmodule; int NO_COPY sigExeced; WCHAR windows_system_directory[MAX_PATH]; diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc index 10910f3ce..2c6910957 100644 --- a/winsup/cygwin/pinfo.cc +++ b/winsup/cygwin/pinfo.cc @@ -49,9 +49,6 @@ 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 @@ -989,22 +986,6 @@ debug_printf ("%d exited buf %d\n", vchild->pid, buf); bool pinfo::wait () { - /* If pending_rd_proc_pipe == NULL we're in an execed process which has - already grabbed the read end of the pipe from the previous cygwin process - running with this pid. */ - if (pending_rd_proc_pipe) - { - /* Our end of the pipe, previously set in prefork() . */ - rd_proc_pipe = pending_rd_proc_pipe; - pending_rd_proc_pipe = NULL; - - /* This sets wr_proc_pipe in the child which, after the following - ForceCloseHandle1, will be only process with the handle open. */ - 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 */ waiter_ready = false; @@ -1022,54 +1003,6 @@ pinfo::wait () return true; } -void -pinfo::prefork (bool detached) -{ - 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 (!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); - pending_rd_proc_pipe = NULL; - } - if (pending_wr_proc_pipe) - { - ForceCloseHandle1 (pending_wr_proc_pipe, wr_proc_pipe); - pending_wr_proc_pipe = NULL; - } -} - -void -pinfo::postexec () -{ - if (wr_proc_pipe () && wr_proc_pipe () != INVALID_HANDLE_VALUE - && !ForceCloseHandle1 (wr_proc_pipe (), 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 in the child. */ bool @@ -1082,19 +1015,17 @@ _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) - /* no parent */; - else + if (my_wr_proc_pipe) { - if (WriteFile (wr_proc_pipe, &sig, 1, &nb, NULL)) + if (WriteFile (my_wr_proc_pipe, &sig, 1, &nb, NULL)) /* all is well */; else if (GetLastError () != ERROR_BROKEN_PIPE) debug_printf ("sending %d notification to parent failed, %E", sig); else { ppid = 1; - HANDLE closeit = wr_proc_pipe; - wr_proc_pipe = INVALID_HANDLE_VALUE; + HANDLE closeit = my_wr_proc_pipe; + my_wr_proc_pipe = NULL; ForceCloseHandle1 (closeit, wr_proc_pipe); } } diff --git a/winsup/cygwin/pinfo.h b/winsup/cygwin/pinfo.h index 1e514e3c0..1588a4029 100644 --- a/winsup/cygwin/pinfo.h +++ b/winsup/cygwin/pinfo.h @@ -121,7 +121,6 @@ public: HANDLE exec_sendsig; DWORD exec_dwProcessId; public: - HANDLE wr_proc_pipe; friend class pinfo_minimal; }; @@ -140,6 +139,7 @@ public: HANDLE hProcess; HANDLE rd_proc_pipe; pinfo_minimal (): h (NULL), hProcess (NULL), rd_proc_pipe (NULL) {} + void set_rd_proc_pipe (HANDLE& h) {rd_proc_pipe = h; h = NULL;} friend class pinfo; }; @@ -147,8 +147,6 @@ 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; @@ -204,15 +202,10 @@ 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.cc b/winsup/cygwin/sigproc.cc index 73b934b3d..78281792f 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -828,7 +828,8 @@ child_info::child_info (unsigned in_cb, child_info_types chtype, bool need_subproc_ready): cb (in_cb), intro (PROC_MAGIC_GENERIC), magic (CHILD_INFO_MAGIC), type (chtype), cygheap (::cygheap), cygheap_max (::cygheap_max), - flag (0), retry (child_info::retry_count) + flag (0), retry (child_info::retry_count), rd_proc_pipe (NULL), + wr_proc_pipe (NULL) { /* It appears that when running under WOW64 on Vista 64, the first DWORD value in the datastructure lpReserved2 is pointing to (msv_count in @@ -876,14 +877,12 @@ child_info::child_info (unsigned in_cb, child_info_types chtype, child_info::~child_info () { - if (subproc_ready) - CloseHandle (subproc_ready); - if (parent) - CloseHandle (parent); + cleanup (); } child_info_fork::child_info_fork () : - child_info (sizeof *this, _CH_FORK, true) + child_info (sizeof *this, _CH_FORK, true), + forker_finished (NULL) { } @@ -893,7 +892,7 @@ child_info_spawn::child_info_spawn (child_info_types chtype, bool need_subproc_r if (type == _CH_EXEC) { hExeced = NULL; - if (myself->wr_proc_pipe) + if (my_wr_proc_pipe) ev = NULL; else if (!(ev = CreateEvent (&sec_none_nih, false, false, NULL))) api_fatal ("couldn't create signalling event for exec, %E"); @@ -912,6 +911,31 @@ cygheap_exec_info::alloc () + (nprocs * sizeof (children[0]))); } +void +child_info::cleanup () +{ + if (subproc_ready) + { + CloseHandle (subproc_ready); + subproc_ready = NULL; + } + if (parent) + { + CloseHandle (parent); + parent = NULL; + } + if (rd_proc_pipe) + { + ForceCloseHandle (rd_proc_pipe); + rd_proc_pipe = NULL; + } + if (wr_proc_pipe) + { + ForceCloseHandle (wr_proc_pipe); + wr_proc_pipe = NULL; + } +} + void child_info_spawn::cleanup () { @@ -940,6 +964,7 @@ child_info_spawn::cleanup () sync_proc_subproc.release (); } type = _CH_NADA; + child_info::cleanup (); } /* Record any non-reaped subprocesses to be passed to about-to-be-execed @@ -1047,7 +1072,7 @@ child_info::sync (pid_t pid, HANDLE& hProcess, DWORD howlong) { res = true; exit_code = STILL_ACTIVE; - if (type == _CH_EXEC && myself->wr_proc_pipe) + if (type == _CH_EXEC && my_wr_proc_pipe) { ForceCloseHandle1 (hProcess, childhProc); hProcess = NULL; diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index ef0403ec0..f5501a540 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -352,10 +352,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, if (mode == _P_OVERLAY) chtype = _CH_EXEC; else - { - chtype = _CH_SPAWN; - myself.prefork (); - } + chtype = _CH_SPAWN; moreinfo = cygheap_exec_info::alloc (); @@ -616,6 +613,12 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, && fhandler_console::tc_getpgid () != myself->pgid) c_flags |= CREATE_NEW_PROCESS_GROUP; refresh_cygheap (); + + if (chtype == _CH_EXEC) + wr_proc_pipe = my_wr_proc_pipe; + else + prefork (); + /* When ruid != euid we create the new process under the current original account and impersonate in child, this way maintaining the different effective vs. real ids. @@ -737,6 +740,8 @@ loop: myself->exec_sendsig = NULL; } myself->process_state &= ~PID_NOTCYGWIN; + if (wr_proc_pipe == my_wr_proc_pipe) + wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */ res = -1; goto out; } @@ -771,18 +776,6 @@ loop: myself.hProcess = hExeced = pi.hProcess; real_path.get_wide_win32_path (myself->progname); // FIXME: race? 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 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 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. myself.postexec () will close our end of - the pipe. */ - if (!newargv.win16_exe && myself->wr_proc_pipe) - myself.postexec (); pid = myself->pid; if (!iscygwin ()) close_all_files (); @@ -801,6 +794,7 @@ loop: res = -1; goto out; } + child.set_rd_proc_pipe (rd_proc_pipe); child->dwProcessId = pi.dwProcessId; child.hProcess = pi.hProcess; @@ -857,7 +851,7 @@ loop: else { close_all_files (true); - if (!myself->wr_proc_pipe + if (!my_wr_proc_pipe && WaitForSingleObject (pi.hProcess, 0) == WAIT_TIMEOUT) { extern bool is_toplevel_proc; @@ -887,8 +881,6 @@ loop: } out: - if (mode != _P_OVERLAY) - myself.postfork (); this->cleanup (); if (envblock) free (envblock);