Skip to content

Conversation

@giampaolo
Copy link
Contributor

@giampaolo giampaolo commented Jan 27, 2019

"In case of premature failure on X.Close() or os.close(X) the remaining pipes/fds will remain "open". Perhaps it makes sense to use contextlib.ExitStack."

This is a follow up of bpo-35537. Rationale of this change originated during code inspection in #11575 (comment).

@gpshead
Copy link
Member

gpshead commented Jan 27, 2019

I suggest adding the following as a NEWS entry:

"An ExitStack is now used internally within subprocess.POpen to clean up pipe file handles. No behavior change in normal operation. But if closing one handle were ever to cause an exception, the others will now be closed instead of leaked."

I tried, but the blurbit web UI wouldn't let me add it to this PR.
reuse the bpo-35537 issue number, no good reason to file an issue specifically for this minor fix.

@gpshead gpshead merged commit bafa848 into python:master Jan 29, 2019
@giampaolo giampaolo deleted the subprocess_spawn_exitstack branch February 3, 2019 22:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants