[ python-Bugs-892939 ] Race condition in popen2

[SourceForge.net]

Bugs item #892939, was opened at 02/08/04 09:31
Message generated for change (Comment added) made by sf-robot
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=892939&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Python Library
Group: Python 2.3
>Status: Closed
Resolution: Fixed
Priority: 6
Submitted By: Ken McNeil (kenmcneil)
Assigned to: Neal Norwitz (nnorwitz)
Summary: Race condition in popen2

Initial Comment:
The "fix" for bug #761888 created a race condition in
Popen3  . The interaction between wait and _cleanup is
the root of the problem.

def wait(self):
  """Wait for and return the exit status of the child
process."""
  if self.sts < 0:
    pid, sts = os.waitpid(self.pid, 0)
    if pid == self.pid:
      self.sts = sts
  return self.sts

def _cleanup():
    for inst in _active[:]:
        inst.poll()

In wait, between the check of self.sts and the call to
os.waitpid a new Popen3 object can be created in
another thread which will trigger a call to _cleanup.
Since the call to _cleanup polls the process, when the
thread running wait starts back up again it will try to
poll the process using os.waitpid, which will throw an
OSError because os.waitpid has already been called for
the PID indirectly in _cleanup.

A work around is for the caller of wait to catch the
OSError and check the sts field, and if sts is
non-negative then the OSError is most likely because of
this problem and can be ignored. However, sts is
undocumented and should probably stay that way.

My suggestion is that the patch that added _active, 
_cleanup, and all  be removed and a more suitable
mechanism for fixing bug #761888 be found. As it has
been said in the discussion of bug #761888, magically
closing FDs is not a "good thing". It seems to me that
surrounding the call to os.fork with a try/except, and
closing the pipes in the except, would be suitable but
I don't know how this would interact with a failed call
to fork, therefore I wont provide a patch.

----------------------------------------------------------------------

>Comment By: SourceForge Robot (sf-robot)
Date: 04/08/06 19:20

Message:
Logged In: YES 
user_id=1312539

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 03/24/06 20:56

Message:
Logged In: YES 
user_id=33168

Martin and I worked out a patch which should solve this
problem and it was committed.  For more info see bug
1183780,   If this does not solve the problem, change the
status from pending to open.  Otherwise, this bug report
should close automatically in a couple of weeks since it's
pending.

----------------------------------------------------------------------

Comment By: Neal Norwitz (nnorwitz)
Date: 03/23/06 00:47

Message:
Logged In: YES 
user_id=33168

I believe this is basically a duplicate of 1183780.  There
is a patch attached there.  Can you verify if it fixes your
problem?

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=105470&aid=892939&group_id=5470