classification
Title: asyncio: fix and refactor creation of subprocess transports
Type: Stage:
Components: asyncio Versions: Python 3.5, Python 3.4
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: gvanrossum, python-dev, vstinner, yselivanov
Priority: normal Keywords: patch

Created on 2015-01-29 11:56 by vstinner, last changed 2018-09-12 18:32 by yselivanov. This issue is now closed.

Files
File name Uploaded Description Edit
subprocess_wait.patch vstinner, 2015-01-29 11:56 review
Messages (6)
msg234963 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-01-29 11:56
Attached patch fix and refactors the creation of asyncio subprocess transports. It fixes the code handling errors.

Changes:

* Add a wait() method to wait until the child process exit

* The constructor now accepts an optional waiter parameter. The _post_init() coroutine must not be called explicitly anymore. It makes subprocess transports closer to other transports, and it gives more freedom if we want later to change completly how subprocess transports are created.

* close() now kills the process instead of kindly terminate it: the child process may ignore SIGTERM and continue to run. Call explicitly terminate() and wait() if you want to kindly terminate the child process.

* close() now logs a warning in debug mode if the process is still running and needs to be killed

* _make_subprocess_transport() is now fully asynchronous again: if the creation of the transport failed, wait asynchronously for the process eixt. Before the wait was synchronous. This change requires close() to *kill*, and not terminate, the child process.

* Remove the _kill_wait() method, replaced with a more agressive close() method. It fixes _make_subprocess_transport() on error. BaseSubprocessTransport.close() calls the close() method of pipe transports, whereas _kill_wait() closed directly pipes of the subprocess.Popen object without unregistering file descriptors from the selector (which caused severe bugs).

* These changes make subprocess.py much simpler!

wait() is a coroutine, which is something uncommon. Is it ok to start using coroutines in transports, at least for subprocess transports?
msg235000 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-01-29 23:09
New changeset a5efd5021ca1 by Victor Stinner in branch '3.4':
asyncio: sync with Tulip
https://hg.python.org/cpython/rev/a5efd5021ca1
msg235001 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-01-29 23:17
New changeset dadc372f46fa by Victor Stinner in branch '3.4':
Issue #23347, asyncio: Make BaseSubprocessTransport.wait() private
https://hg.python.org/cpython/rev/dadc372f46fa
msg235002 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2015-01-29 23:19
I commited my change with small changes.

"wait() is a coroutine, which is something uncommon. Is it ok to start using coroutines in transports, at least for subprocess transports?"

I made the method private (renamed to _wait()).
msg235008 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-01-30 00:22
New changeset c1d92f7221a5 by Victor Stinner in branch '3.4':
Issue #23347, asyncio: send_signal(), terminate(), kill() don't check if the
https://hg.python.org/cpython/rev/c1d92f7221a5
msg325157 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2018-09-12 18:32
New changeset aca819fb494d4801b3e5b5b507b17cab772c1b40 by Yury Selivanov (Bumsik Kim) in branch 'master':
bpo-33649: Fix doc to reflect changes in 47cd10d (or bpo-23347) (GH-9219)
https://github.com/python/cpython/commit/aca819fb494d4801b3e5b5b507b17cab772c1b40
History
Date User Action Args
2018-09-12 18:32:01yselivanovsetmessages: + msg325157
2015-01-30 00:22:08python-devsetmessages: + msg235008
2015-01-29 23:19:29vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg235002
2015-01-29 23:17:44python-devsetmessages: + msg235001
2015-01-29 23:09:02python-devsetnosy: + python-dev
messages: + msg235000
2015-01-29 11:56:49vstinnercreate