Close stdin immediately when using popen2e#9540
Merged
Merged
Conversation
kou
reviewed
May 12, 2026
d4062de to
5b3f795
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Gem::Ext::Builder.run to proactively close the child process STDIN pipe created by Open3.popen2e, preventing subprocess trees from blocking on STDIN reads during native extension builds (e.g., extconf.rb → cargo build → credential helper).
Changes:
- Close the
stdinIO immediately inside theOpen3.popen2eblock inGem::Ext::Builder.run. - Add a regression test asserting that
Gem::Ext::Builder.runprovides an immediately-EOF STDIN to the child process.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/rubygems/ext/builder.rb | Closes stdin immediately after spawning the build command to avoid hangs from downstream STDIN reads. |
| test/rubygems/test_gem_ext_builder.rb | Adds a test that verifies STDIN is closed (EOF) for commands run via Gem::Ext::Builder.run. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+119
to
+123
| Gem::Ext::Builder.run([Gem.ruby, "-e", check_stdin_script], results) | ||
|
|
||
| command_output = results.last | ||
| assert_equal "STDIN: \"\"\n", command_output | ||
| end |
hsbt
approved these changes
May 14, 2026
Member
|
@rwstauner Thanks always. I'm not sure why |
5b3f795 to
69a4ff1
Compare
It's good hygiene to close the stdin pipe as soon as you are done writing to it. This can happen for example if "ruby extconf.rb" spawns another process (for example "cargo build", which may spawn arbitrary commands to fetch credentials) and any of those subprocesses attempt to read STDIN until it is closed. We can close it as soon as it's created since we aren't writing to it at all.
69a4ff1 to
ab09bfd
Compare
Contributor
Author
|
I rebased and it's green now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It's good hygiene to close the stdin pipe as soon as you are done writing to it.
This can happen for example if "ruby extconf.rb" spawns another process (for example "cargo build", which may spawn arbitrary commands to fetch credentials) and any of those subprocesses attempt to read STDIN until it is closed.
We can close it as soon as it's created since we aren't writing to it at all.
What was the end-user or developer problem that led to this PR?
Our CI was hanging and timing out at the "bundle install" step because a gem's extconf.rb calls
cargo buildwhich calls a custom command to fetch credentials and that command tries to read STDIN when it detects that it is given a pipe.What is your fix for the problem, implemented in this PR?
Close the STDIN pipe created by Open3 so that if a target command tries to read the pipe it will finish.
Make sure the following tasks are checked