Skip to content

Fix pipe reading hanging indefinitely on Windows#756

Merged
woodruffw merged 4 commits intopypa:mainfrom
mathbou:subprocess-hangs-indefinitely
Apr 12, 2024
Merged

Fix pipe reading hanging indefinitely on Windows#756
woodruffw merged 4 commits intopypa:mainfrom
mathbou:subprocess-hangs-indefinitely

Conversation

@mathbou
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
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