Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asyncio.subprocess's communicate() method mishandles empty input bytes #71035

Closed
oconnor663 mannequin opened this issue Apr 26, 2016 · 5 comments
Closed

asyncio.subprocess's communicate() method mishandles empty input bytes #71035

oconnor663 mannequin opened this issue Apr 26, 2016 · 5 comments
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@oconnor663
Copy link
Mannequin

oconnor663 mannequin commented Apr 26, 2016

BPO 26848
Nosy @gvanrossum, @vstinner, @berkerpeksag, @1st1, @oconnor663

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 2016-05-14.01:42:18.664>
created_at = <Date 2016-04-26.04:40:33.845>
labels = ['type-bug', 'expert-asyncio']
title = "asyncio.subprocess's communicate() method mishandles empty input bytes"
updated_at = <Date 2016-05-14.01:42:18.663>
user = 'https://github.com/oconnor663'

bugs.python.org fields:

activity = <Date 2016-05-14.01:42:18.663>
actor = 'berker.peksag'
assignee = 'none'
closed = True
closed_date = <Date 2016-05-14.01:42:18.664>
closer = 'berker.peksag'
components = ['asyncio']
creation = <Date 2016-04-26.04:40:33.845>
creator = 'oconnor663'
dependencies = []
files = []
hgrepos = []
issue_num = 26848
keywords = []
message_count = 5.0
messages = ['264212', '264213', '264214', '264215', '265487']
nosy_count = 6.0
nosy_names = ['gvanrossum', 'vstinner', 'python-dev', 'berker.peksag', 'yselivanov', 'oconnor663']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26848'
versions = ['Python 3.5', 'Python 3.6']

@oconnor663
Copy link
Mannequin Author

oconnor663 mannequin commented Apr 26, 2016

Setting stdin=PIPE and then calling communicate(b"") should close the child's stdin immediately, similar to stdin=DEVNULL. Instead, communicate() treats b"" like None and leaves the child's stdin open, which makes the child hang forever if it tries to read anything.

I have a PR open with a fix and a test: #33

@oconnor663 oconnor663 mannequin added topic-asyncio type-bug An unexpected behavior, bug, or error labels Apr 26, 2016
@berkerpeksag
Copy link
Member

python/cpython is a semi-official read-only mirror of hg.python.org/cpython. We haven't switched to GitHub yet. You may want to open a pull request to https://github.com/python/asyncio See https://github.com/python/asyncio/wiki/Contributing for details.

@oconnor663
Copy link
Mannequin Author

oconnor663 mannequin commented Apr 26, 2016

Thanks for the heads up, Berker, I've re-submitted the PR as python/asyncio#335.

@oconnor663
Copy link
Mannequin Author

oconnor663 mannequin commented Apr 26, 2016

Related: The asyncio communicate() method differs from standard subprocess in how it treats input bytes when stdin is (probably mistakenly) not set to PIPE. Like this:

    proc = await create_subprocess_shell("sleep 5")
    await proc.communicate(b"foo")  # Oops, I forgot stdin=PIPE above!

The standard, non-async version of this example, communicate would ignore the input bytes entirely. But here in the asyncio version, communicate will try to write those bytes to stdin, which is None, and the result is an AttributeError.

Since the user probably only hits this case by mistake, I think raising an exception is preferable. But it would be nice to raise an exception that explicitly said "you've forgotten stdin=PIPE" instead of the unhelpful "'NoneType' object has no attribute 'write'". Maybe it would be worth cleaning this up while we're here?

@python-dev
Copy link
Mannequin

python-dev mannequin commented May 13, 2016

New changeset 148757a88f19 by Yury Selivanov in branch '3.5':
Issue bpo-26848: Fix asyncio/subprocess.communicate() to handle empty input.
https://hg.python.org/cpython/rev/148757a88f19

New changeset f5e69e2f50d7 by Yury Selivanov in branch 'default':
Merge 3.5 (issue bpo-26848)
https://hg.python.org/cpython/rev/f5e69e2f50d7

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant