Issue33319
Created on 2018-04-20 13:05 by pekka.klarck, last changed 2022-04-11 14:58 by admin. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 12508 | merged | gregory.p.smith, 2019-03-23 07:25 | |
| PR 12509 | merged | miss-islington, 2019-03-23 07:40 | |
| Messages (6) | |||
|---|---|---|---|
| msg315510 - (view) | Author: Pekka Klärck (pekka.klarck) | Date: 2018-04-20 13:05 | |
I'm porting old scripts from Python 2.7 to 3.6 and plan to change `subprocess.call()` to `subprocess.run()` at the same time. When using `call()` I've used `tempfile.TemporaryFile` as stdout because it's documentation has this warning:
Note: Do not use stdout=PIPE or stderr=PIPE with this function. The child process will block if it generates enough output to a pipe to fill up the OS pipe buffer as the pipes are not being read from.
Interestingly there is no such note in the docs of `run()`, and based on my (possibly inadequate) testing I couldn't get it to hang either. I'm still somewhat worried about using `stdout=PIPE` with it because the docs don't explicitly say it would be safe. I'm especially worried because the docs of `call()` nowadays say that it's equivalent to `run(...).returncode`. If that's the case, then I would expect the warning in `call()` to apply also to `run()`. Or is the warning nowadays outdated altogether?
|
|||
| msg315526 - (view) | Author: Josh Rosenberg (josh.r) * ![]() |
Date: 2018-04-20 17:26 | |
If the goal is just to suppress stdout, that's what passing subprocess.DEVNULL is for (doesn't exist in Py2, but opening os.devnull and passing that is a slightly higher overhead equivalent). subprocess.run includes a call to communicate as part of its default behavior, and stores its results, so call() isn't quite equivalent to run().returncode when PIPE was passed for standard handles, because call only includes an implicit call to wait, not communicate, and therefore pipes are not explicitly read and can block. Basically, subprocess.run is deadlock-safe (because it uses communicate, not just wait), but if you don't care about the results, and the results might be huge, don't pass it PIPE for stdout/stderr (because it will store the complete outputs in memory, just like any use of communicate with PIPE). The docs effectively tell you PIPE is safe; it returns a CompletedProcess object, and explicitly tells you that it has attributes that are (completely) populated based on whether capture was requested. If it had such attributes and still allowed deadlocks, it would definitely merit a warning. |
|||
| msg315651 - (view) | Author: Pekka Klärck (pekka.klarck) | Date: 2018-04-23 08:23 | |
My goal is to read stdout. It's good to hear `subprocess.run()` is deadlock-safe and I can use it safely. Making the docs explicit about it so that others know it's safe would in my opinion be a good idea as well. Casual users don't know `run()` it uses `communicate()`, not `wait()`, internally, or even that this would mean it cannot deadlock. The current situation when the docs say that `call()` shouldn't be used with `stdout=PIPE` and that `call(...)` is equivalent to `run(...).returncode` indicates `stdout=PIPE` is unsafe with `run()` as well. A separate questions is that if `call(...)` is equivalent to `run(...).returncode`, should it also be implemented that way. Based on this discussion it would avoid the problem with `stdout=PIPE` also in that case. |
|||
| msg338636 - (view) | Author: Martin Panter (martin.panter) * ![]() |
Date: 2019-03-23 00:53 | |
This is a regression in the 3.7+ documentation. It previously said “To [capture output], pass PIPE for the ‘stdout’ and/or ‘stderr’ arguments”. This was removed by Bo Bayles in Issue 32102. |
|||
| msg338652 - (view) | Author: Gregory P. Smith (gregory.p.smith) * ![]() |
Date: 2019-03-23 07:40 | |
New changeset 7a2e84c3488cfd6c108c6b41ff040825f1757566 by Gregory P. Smith in branch 'master': bpo-33319: Clarify subprocess call docs. (GH-12508) https://github.com/python/cpython/commit/7a2e84c3488cfd6c108c6b41ff040825f1757566 |
|||
| msg338654 - (view) | Author: miss-islington (miss-islington) | Date: 2019-03-23 07:46 | |
New changeset 9cdac5ced68f1d6ef5e1eee7552bb200b46adc23 by Miss Islington (bot) in branch '3.7': bpo-33319: Clarify subprocess call docs. (GH-12508) https://github.com/python/cpython/commit/9cdac5ced68f1d6ef5e1eee7552bb200b46adc23 |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:59 | admin | set | github: 77500 |
| 2019-03-23 07:51:13 | gregory.p.smith | set | status: open -> closed nosy: - miss-islington resolution: fixed |
| 2019-03-23 07:46:17 | miss-islington | set | nosy:
+ miss-islington messages: + msg338654 |
| 2019-03-23 07:40:47 | miss-islington | set | pull_requests: + pull_request12460 |
| 2019-03-23 07:40:33 | gregory.p.smith | set | messages: + msg338652 |
| 2019-03-23 07:25:47 | gregory.p.smith | set | keywords:
+ patch stage: needs patch -> patch review pull_requests: + pull_request12459 |
| 2019-03-23 00:53:20 | martin.panter | set | versions:
+ Python 3.7, Python 3.8, Python 3.9 nosy: + martin.panter, bbayles, gregory.p.smith messages: + msg338636 keywords:
+ 3.7regression |
| 2018-04-23 08:23:44 | pekka.klarck | set | messages: + msg315651 |
| 2018-04-20 17:26:03 | josh.r | set | nosy:
+ josh.r messages: + msg315526 |
| 2018-04-20 13:05:37 | pekka.klarck | set | assignee: docs@python components:
+ Documentation |
| 2018-04-20 13:05:14 | pekka.klarck | create | |

