Skip to content

Commit e1fafcb

Browse files
authored
Fix subprocess.communicate("") raising IOError: closed stream (#78)
`subprocess.communicate("")` would raise `IOError: closed stream` because the code closed stdin immediately when `input.empty?`, but then later tried to use the closed stdin file descriptor in `IO.select` This only occurs with empty strings (`""`), not with `nil` or non-empty strings. We've fixed that by excluding `@stdin` from `wait_w` when `@stdin.closed?` ### Backward compatibility This fix maintains full backward compatibility: - `nil` input behaviour unchanged - Non-empty string input behaviour unchanged - Only empty string input is fixed (was previously broken)
1 parent e5ceb8b commit e1fafcb

File tree

2 files changed

+57
-2
lines changed

2 files changed

+57
-2
lines changed

lib/subprocess.rb

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -436,13 +436,15 @@ def communicate(input=nil, timeout_s=nil)
436436
# the input depending on how many bytes were written
437437
input = input.dup.force_encoding('BINARY') unless input.nil?
438438

439-
@stdin.close if (input.nil? || input.empty?) && !@stdin.nil?
439+
# Close stdin immediately if input is nil or empty
440+
@stdin.close if @stdin && (input.nil? || input.empty?)
440441

441442
timeout_at = Time.now + timeout_s if timeout_s
442443

443444
self.class.catching_sigchld(pid) do |global_read, self_read|
444445
wait_r = [@stdout, @stderr, self_read, global_read].compact
445-
wait_w = [input && @stdin].compact
446+
wait_w = @stdin&.closed? ? [] : [input && @stdin].compact
447+
446448
done = false
447449
while !done
448450
# If the process has exited, we want to drain any remaining output before returning

test/test_empty_stdin.rb

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
# frozen_string_literal: true
2+
require 'rubygems'
3+
gem 'minitest'
4+
require 'minitest/autorun'
5+
require 'subprocess'
6+
7+
describe Subprocess do
8+
describe "communicate with empty string input" do
9+
# Bug report: subprocess.communicate("") doesn't properly handle stdin,
10+
# causing it to close incorrectly and result in a broken pipe.
11+
it "should not raise IOError when passing empty string" do
12+
# Before the fix, this would raise: IOError: closed stream
13+
Subprocess.check_call(['cat'],
14+
stdin: Subprocess::PIPE,
15+
stdout: Subprocess::PIPE) do |p|
16+
stdout, stderr = p.communicate("")
17+
assert_equal("", stdout, "Empty input should produce empty output")
18+
assert_equal("", stderr, "No errors expected")
19+
end
20+
end
21+
22+
it "should work correctly with non-empty string input" do
23+
test_input = "hello world"
24+
Subprocess.check_call(['cat'],
25+
stdin: Subprocess::PIPE,
26+
stdout: Subprocess::PIPE) do |p|
27+
stdout, stderr = p.communicate(test_input)
28+
assert_equal(test_input, stdout, "Input should be echoed back")
29+
assert_equal("", stderr, "No errors expected")
30+
end
31+
end
32+
33+
it "should work correctly with nil input" do
34+
Subprocess.check_call(['cat'],
35+
stdin: Subprocess::PIPE,
36+
stdout: Subprocess::PIPE) do |p|
37+
stdout, stderr = p.communicate(nil)
38+
assert_equal("", stdout, "Nil input should produce empty output")
39+
assert_equal("", stderr, "No errors expected")
40+
end
41+
end
42+
43+
it "should handle already closed stdin gracefully" do
44+
# Edge case: what if stdin is already closed?
45+
p = Subprocess.popen(['cat'], stdin: Subprocess::PIPE, stdout: Subprocess::PIPE)
46+
p.stdin.close
47+
stdout, stderr = p.communicate("")
48+
assert_equal("", stdout)
49+
assert_equal("", stderr)
50+
p.wait
51+
end
52+
end
53+
end

0 commit comments

Comments
 (0)