* 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.
This commit is contained in:
Christopher Faylor 2012-05-08 15:06:43 +00:00
parent 51180c08ed
commit dfd5d5bea6
5 changed files with 60 additions and 8 deletions

View File

@ -1,3 +1,13 @@
2012-05-08 Christopher Faylor <me.cygwin2012@cgf.cx>
* 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 <me.cygwin2012@cgf.cx> 2012-05-07 Christopher Faylor <me.cygwin2012@cgf.cx>
* DevNotes: Add entry cgf-000003. * DevNotes: Add entry cgf-000003.

View File

@ -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> <1.7.15>
Don't make Cygwin wait for all children of a non-cygwin child program. Don't make Cygwin wait for all children of a non-cygwin child program.

View File

@ -393,8 +393,8 @@ frok::parent (volatile char * volatile stack_here)
/* Wait for subproc to initialize itself. */ /* Wait for subproc to initialize itself. */
if (!ch.sync (pi.dwProcessId, hchild, FORK_WAIT_TIMEOUT)) if (!ch.sync (pi.dwProcessId, hchild, FORK_WAIT_TIMEOUT))
{ {
if (!error ("forked process died unexpectedly, retry %d, exit code %d", if (!error ("forked process %u died unexpectedly, retry %d, exit code %d",
ch.retry, ch.exit_code)) pi.dwProcessId, ch.retry, ch.exit_code))
continue; continue;
this_errno = EAGAIN; this_errno = EAGAIN;
goto cleanup; goto cleanup;

View File

@ -300,6 +300,13 @@ pinfo::init (pid_t n, DWORD flag, HANDLE h0)
bool created = shloc != SH_JUSTOPEN; 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) if ((procinfo->process_state & PID_REAPED)
|| ((procinfo->process_state & PID_INITIALIZING) && (flag & PID_NOREDIR) || ((procinfo->process_state & PID_INITIALIZING) && (flag & PID_NOREDIR)
&& cygwin_pid (procinfo->dwProcessId) != procinfo->pid)) && cygwin_pid (procinfo->dwProcessId) != procinfo->pid))

View File

@ -422,8 +422,8 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
moreinfo->argc = newargv.argc; moreinfo->argc = newargv.argc;
moreinfo->argv = newargv; moreinfo->argv = newargv;
if (mode != _P_OVERLAY || if (mode != _P_OVERLAY || !real_path.iscygexec ()
!DuplicateHandle (GetCurrentProcess (), myself.shared_handle (), || !DuplicateHandle (GetCurrentProcess (), myself.shared_handle (),
GetCurrentProcess (), &moreinfo->myself_pinfo, GetCurrentProcess (), &moreinfo->myself_pinfo,
0, TRUE, DUPLICATE_SAME_ACCESS)) 0, TRUE, DUPLICATE_SAME_ACCESS))
moreinfo->myself_pinfo = NULL; moreinfo->myself_pinfo = NULL;
@ -740,7 +740,7 @@ loop:
process fails. Only need to do this for _P_OVERLAY since the handle will 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 otherwise. Don't need to do this for 'parent' since it will
be closed in every case. See FIXME above. */ 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); SetHandleInformation (wr_proc_pipe, HANDLE_FLAG_INHERIT, HANDLE_FLAG_INHERIT);
if (wr_proc_pipe == my_wr_proc_pipe) if (wr_proc_pipe == my_wr_proc_pipe)
wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */ wr_proc_pipe = NULL; /* We still own it: don't nuke in destructor */