stderr FDs have been inherited by a child process (original) (raw)

(This is a spinoff from http://bugs.python.org/issue19293#msg200598)

When SIGCHLD is received, _sig_chld() is executed:

def _sig_chld(self):
    [...]
    transp = self._subprocesses.get(pid)
    if transp is not None:
        transp._process_exited(returncode)

Then, here's _process_exited():

def _process_exited(self, returncode):
    assert returncode is not None, returncode
    assert self._returncode is None, self._returncode
    self._returncode = returncode
    self._loop._subprocess_closed(self)
    self._call(self._protocol.process_exited)
    self._try_finish()

And here's _try_finish():

def _try_finish(self):
    assert not self._finished
    if self._returncode is None:
        return
    if all(p is not None and p.disconnected
           for p in self._pipes.values()):
        self._finished = True
        self._loop.call_soon(self._call_connection_lost, None)

Thus, _UnixSubprocessTransport protocol's connection_lost is only called if the all() expression is true: and it's true only if all the subprocess pipes have been disconnected (or if we didn't setup any pipe).

Unfortunately, this might very well never happen: imagine that the subprocess forks a process: this grand-child process inherits the child process's pipes, so when the child process exits, we won't receive any notification, since this grand-child process still has open FDs pointing to the original child's stdin/stdout/stderr.

The following simple test will hang until the background 'sleep' exits: """ diff -r 47618b00405b Lib/test/test_asyncio/test_events.py --- a/Lib/test/test_asyncio/test_events.py Sat Oct 19 10:45:48 2013 +0300 +++ b/Lib/test/test_asyncio/test_events.py Sun Oct 20 19:32:37 2013 +0200 @@ -1059,6 +1059,23 @@

 @unittest.skipIf(sys.platform == 'win32',
                  "Don't support subprocess for Windows yet")

def test_subprocess_inherit_fds(self):
```
   proto = None
```
```
   @tasks.coroutine
```
```
   def connect():
```
```
       nonlocal proto
```

       transp, proto = yield from self.loop.subprocess_shell(

           functools.partial(MySubprocessProtocol, self.loop),

```
           'sleep 60 &')
```

       self.assertIsInstance(proto, MySubprocessProtocol)

   self.loop.run_until_complete(connect())

   self.loop.run_until_complete(proto.completed)

   self.assertEqual(0, proto.returncode)

@unittest.skipIf(sys.platform == 'win32',
```
                "Don't support subprocess for Windows yet")
```
def test_subprocess_close_after_finish(self): proto = None transp = None

"""

If waitpid() returns a process's PID, then the process is done, there's no reason to further wait for pipe's disconnection: they can be used as a hint that the process terminated, but there's definitely not required...

This is by design. Let me try to defend the design.

As long as one of the pipes is still open the parent might be interested in it. The Protocol object does get notified of the process's exit, via process_exited(), and if at that point it wants to be done, it can close the pipes itself. (To do that, call transport.get_pipe_transport(FD).close().) Once that's done the subprocess protocol's connection_lost() method will be called.

I suppose an alternative approach might be to assume that when the subprocess exist the parent should close the pipes and be done, but there's a race condition there: there may still be data in one of the pipes (stdout or stderr) that should be processed before calling connection_lost(), similar to how we delay the socket connection_lost() call until we have processed all data read from it.

So I don't think that alternative is viable.

This is by design. Let me try to defend the design.

OK, if that's a know limitation, then that's fine. It would be nice to add maybe a note somewhere in the documentation, so that people don't get bitten by this (and also probably add a test for process_exited).