[issue19326] asyncio: child process exit isn't detected if its stdin/stdout/stderr FDs have been inherited by a child process

2013-10-21 Thread Charles-François Natali

New submission from Charles-François Natali:

(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...

--
messages: 200744
nosy: neologix
priority: normal
severity: normal
stage: needs patch
status: open
title: asyncio: child process exit isn't detected if its stdin/stdout/stderr 
FDs have been inherited by a child process
type: behavior
versions: Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19326
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19326] asyncio: child process exit isn't detected if its stdin/stdout/stderr FDs have been inherited by a child process

2013-10-21 Thread Guido van Rossum

Changes by Guido van Rossum gu...@python.org:


--
nosy: +gvanrossum

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19326
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19326] asyncio: child process exit isn't detected if its stdin/stdout/stderr FDs have been inherited by a child process

2013-10-21 Thread Guido van Rossum

Guido van Rossum added the comment:

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.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19326
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com



[issue19326] asyncio: child process exit isn't detected if its stdin/stdout/stderr FDs have been inherited by a child process

2013-10-21 Thread Charles-François Natali

Charles-François Natali added the comment:

 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).

--
resolution:  - rejected
stage: needs patch - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue19326
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com