From b79c0094f8a9bee17aacd59fe7e0334228a8fae8 Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Wed, 2 May 2012 16:39:39 +0000 Subject: [PATCH] * ChangeNotes: New file. Add entry cgf-000001. * sigproc.cc (proc_terminate): Don't set parent pid of child to 1 if we've execed since the execed process is still considered the parent. * child_info.h: Bump copyright. --- winsup/cygwin/ChangeLog | 8 ++++++++ winsup/cygwin/DevNotes | 26 ++++++++++++++++++++++++++ winsup/cygwin/child_info.h | 2 +- winsup/cygwin/sigproc.cc | 10 +++++++++- 4 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 winsup/cygwin/DevNotes diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 34afa5b85..9cda5ed55 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,11 @@ +2012-05-02 Christopher Faylor + + * ChangeNotes: New file. Add entry cgf-000001. + * sigproc.cc (proc_terminate): Don't set parent pid of child to 1 if + we've execed since the execed process is still considered the parent. + + * child_info.h: Bump copyright. + 2012-05-02 Corinna Vinschen * fenv.cc (fesetround): Fix test for valid input parameter. diff --git a/winsup/cygwin/DevNotes b/winsup/cygwin/DevNotes new file mode 100644 index 000000000..da3edb048 --- /dev/null +++ b/winsup/cygwin/DevNotes @@ -0,0 +1,26 @@ +2012-05-02 cgf-000001 + +This fix was due to a bug report on the Cygwin mailing list: +http://cygwin.com/ml/cygwin/2012-05/msg00009.html + +strace showed that ssh-agent was checking the parent pid and getting a 1 +when it shouldn't have. Other stuff looked ok so I chose to consider +this a smoking gun. + +Going back to the version that the OP said did not have the problem, I +worked forward until I found where the problem first occurred - +somewhere around 2012-03-19. And, indeed, the getppid call returned the +correct value in the working version. That means that this stopped +working when I redid the way the process pipe was inherited around +this time period. + +It isn't clear why (and I suspect I may have to debug this further at +some poit) this hasn't always been a problem but I made the obvious fix. +We shouldn't have been setting ppid = 1 when we're about to pass off to +an execed process. + +As I was writing this, I realized that it was necessary to add some +additional checks. Just checking for "have_execed" isn't enough. If +we've execed a non-cygwin process then it won't know how to deal with +any inherited children. So, always set ppid = 1 if we've execed a +non-cygwin process. diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index 36aac8ed1..36a22539c 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -1,6 +1,6 @@ /* child_info.h: shared child info for cygwin - Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008, 2009, 2011 + Copyright 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2008, 2009, 2011, 2012 Red Hat, Inc. This file is part of Cygwin. diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index c9a76d85f..05d98729f 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -445,9 +445,17 @@ proc_terminate () /* Clean out proc processes from the pid list. */ for (int i = 0; i < nprocs; i++) { - procs[i]->ppid = 1; + /* If we've execed then the execed process will handle setting ppid + to 1 iff it is a Cygwin process. */ + if (!have_execed || !have_execed_cygwin) + procs[i]->ppid = 1; if (procs[i].wait_thread) procs[i].wait_thread->terminate_thread (); + /* Release memory associated with this process unless it is 'myself'. + 'myself' is only in the procs table when we've execed. We reach + here when the next process has finished initializing but we still + can't free the memory used by 'myself' since it is used later on + during cygwin tear down. */ if (procs[i] != myself) procs[i].release (); }