diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 0e3056e73..760df768c 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,13 @@ +2012-05-08 Christopher Faylor + + * DevNotes: Add entry cgf-000004. + * pinfo.cc (pinfo::init): Reuse shared memory if the state is marked + with PID_REAPED. + * spawn.cc (child_info_spawn::worker): Don't duplicate myself_pinfo + into non-cygwin child. + + * fork.cc (frok::parent): Improve error output. + 2012-05-07 Christopher Faylor * DevNotes: Add entry cgf-000003. diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes index b9d031f1e..83be5e9cd 100644 --- a/winsup/cygwin/DevNotes +++ b/winsup/cygwin/DevNotes @@ -1,4 +1,39 @@ -2012-05-03 cgf-000003 +2012-05-08 cgf-000004 + +The change associated with cgf-000003 introduced a new problem. + +Since a handle associated with the parent is no longer being duplicated +into a non-cygwin "execed child", Windows is free to reuse the pid of +the parent when the parent exits. However, since we *did* duplicate a +handle pointing to the pid's shared memory area into the "execed child", +the shared memory for the pid was still active. + +Since the shared memory was still available, if a new process reuses the +previous pid, Cygwin would detect that the shared memory was not created +and had a "PID_REAPED" flag. That was considered an error, and, so, it +would set procinfo to NULL and pinfo::thisproc would die since this +situation is not supposed to occur. + +I fixed this in two ways: + +1) If a shared memory region has a PID_REAPED flag then zero it and +reuse it. This should be safe since you are not really supposed to be +querying the shared memory region for anything after PID_REAPED has been +set. + +2) Forego duping a copy of myself_pinfo if we're starting a non-cygwin +child for exec. + +It seems like 2) is a common theme and an audit of all of the handles +that are being passed to non-cygwin children is in order for 1.7.16. + +The other minor modification that was made in this change was to add the +pid of the failing process to fork error output. This helps slightly +when looking at strace output, even though in this case it was easy to +find what was failing by looking for '^---' when running the "stv" +strace dumper. That found the offending exception quickly. + +2012-05-07 cgf-000003 <1.7.15> Don't make Cygwin wait for all children of a non-cygwin child program. diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc index e617bd133..22076fc1d 100644 --- a/winsup/cygwin/fork.cc +++ b/winsup/cygwin/fork.cc @@ -393,8 +393,8 @@ frok::parent (volatile char * volatile stack_here) /* Wait for subproc to initialize itself. */ if (!ch.sync (pi.dwProcessId, hchild, FORK_WAIT_TIMEOUT)) { - if (!error ("forked process died unexpectedly, retry %d, exit code %d", - ch.retry, ch.exit_code)) + if (!error ("forked process %u died unexpectedly, retry %d, exit code %d", + pi.dwProcessId, ch.retry, ch.exit_code)) continue; this_errno = EAGAIN; goto cleanup; diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc index daed1cc7b..fda0abc9a 100644 --- a/winsup/cygwin/pinfo.cc +++ b/winsup/cygwin/pinfo.cc @@ -300,6 +300,13 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0) bool created = shloc != SH_JUSTOPEN; + if (!created && createit && (procinfo->process_state & PID_REAPED)) + { + memset (procinfo, 0, sizeof (*procinfo)); + created = true; /* Lie that we created this - just reuse old + shared memory */ + } + if ((procinfo->process_state & PID_REAPED) || ((procinfo->process_state & PID_INITIALIZING) && (flag & PID_NOREDIR) && cygwin_pid (procinfo->dwProcessId) != procinfo->pid)) diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index 8b5e382c5..7e165cf8b 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -422,10 +422,10 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv, moreinfo->argc = newargv.argc; moreinfo->argv = newargv; - if (mode != _P_OVERLAY || - !DuplicateHandle (GetCurrentProcess (), myself.shared_handle (), - GetCurrentProcess (), &moreinfo->myself_pinfo, - 0, TRUE, DUPLICATE_SAME_ACCESS)) + if (mode != _P_OVERLAY || !real_path.iscygexec () + || !DuplicateHandle (GetCurrentProcess (), myself.shared_handle (), + GetCurrentProcess (), &moreinfo->myself_pinfo, + 0, TRUE, DUPLICATE_SAME_ACCESS)) moreinfo->myself_pinfo = NULL; else VerifyHandle (moreinfo->myself_pinfo); @@ -740,7 +740,7 @@ loop: process fails. Only need to do this for _P_OVERLAY since the handle will be closed otherwise. Don't need to do this for 'parent' since it will be closed in every case. See FIXME above. */ - if (!real_path.iscygexec () && mode == _P_OVERLAY) + if (!iscygwin () && mode == _P_OVERLAY) SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT); if (wr_proc_pipe == my_wr_proc_pipe) wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */