subprocess.run documentation doesn't tell is using stdout=PIPE safe

[pekkaklarck]

BPO	33319
Nosy	@gpshead, @vadmium, @MojoVampire, @bbayles
PRs	bpo-33319: Clarify subprocess call docs. #12508 [3.7] bpo-33319: Clarify subprocess call docs. (GH-12508) #12509

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-03-23.07:51:13.282>
created_at = <Date 2018-04-20.13:05:14.825>
labels = ['3.7', '3.8', '3.9', 'docs']
title = "`subprocess.run` documentation doesn't tell is using `stdout=PIPE` safe"
updated_at = <Date 2019-03-23.07:51:13.278>
user = 'https://bugs.python.org/pekkaklarck'

bugs.python.org fields:

activity = <Date 2019-03-23.07:51:13.278>
actor = 'gregory.p.smith'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-03-23.07:51:13.282>
closer = 'gregory.p.smith'
components = ['Documentation']
creation = <Date 2018-04-20.13:05:14.825>
creator = 'pekka.klarck'
dependencies = []
files = []
hgrepos = []
issue_num = 33319
keywords = ['patch', '3.7regression']
message_count = 6.0
messages = ['315510', '315526', '315651', '338636', '338652', '338654']
nosy_count = 6.0
nosy_names = ['gregory.p.smith', 'docs@python', 'pekka.klarck', 'martin.panter', 'josh.r', 'bbayles']
pr_nums = ['12508', '12509']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue33319'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

[pekkaklarck]

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?

[MojoVampire]

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.

[pekkaklarck]

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.

[vadmium]

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 bpo-32102.

[gpshead]

New changeset 7a2e84c by Gregory P. Smith in branch 'master':
bpo-33319: Clarify subprocess call docs. (GH-12508)
7a2e84c

[miss-islington]

New changeset 9cdac5c by Miss Islington (bot) in branch '3.7':
bpo-33319: Clarify subprocess call docs. (GH-12508)
9cdac5c