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

Fix pipe reading hanging indefinitely on Windows #756

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

mathbou
Copy link
Contributor

@mathbou mathbou commented Apr 1, 2024

While doing tests with pip-audit, I noticed several times the process stalling while installing the isolated env.

By removing the stdout/stderr read size, it seems to run properly, however it's quite tricky to reproduce.

@woodruffw
Copy link
Member

I'm acknowledging this PR so you know I'm not ignoring it, but JFYI: I'm pretty backlogged at the moment, so I can't guarantee a timely review here 🙂

@woodruffw woodruffw added bug Something isn't working plat:windows Issues reported on Windows labels Apr 3, 2024
@woodruffw
Copy link
Member

Took a look, and I have no major objection to removing the explicit buffer sizes. That being said, it'd be ideal to have a better understanding of why this would deadlock on Windows: we set bufsize=0 so the pipe should be unbuffered regardless of OS, meaning that neither read() should ever block.

@woodruffw woodruffw enabled auto-merge (squash) April 12, 2024 20:22
@woodruffw woodruffw merged commit 3f52615 into pypa:main Apr 12, 2024
7 checks passed
woodruffw added a commit that referenced this pull request Apr 12, 2024
Signed-off-by: William Woodruff <william@trailofbits.com>
@mathbou mathbou deleted the subprocess-hangs-indefinitely branch April 13, 2024 11:46
woodruffw added a commit that referenced this pull request Apr 30, 2024
Signed-off-by: William Woodruff <william@trailofbits.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working plat:windows Issues reported on Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants