Skip to content

Commit 4f7048d

Browse files
Fix Process#wait to not close @input (#16620)
Prior to this change, `Process#wait` would close the input stream if it is a pipe. This is an unexpected side effect. It prevents writing to the input while the process is still running. When using `#wait` explicitly, it's now the user's responsibility to close the input pipe at an appropriate time if necessary. We keep the closing behaviour in the yielding version of `Process.run` where it makes sense: Everything during the runtime of the process is expected to be inside the block. It even expands to closing all streams (if they're a pipe). This was already stated in the API docs, but effectively only input was closed.
1 parent 4032543 commit 4f7048d

File tree

3 files changed

+49
-3
lines changed

3 files changed

+49
-3
lines changed

spec/std/process_spec.cr

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ require "spec"
44
require "process"
55
require "./spec_helper"
66
require "../support/env"
7+
require "../support/wait_for"
78

89
private def exit_code_command(code)
910
{% if flag?(:win32) %}
@@ -214,11 +215,40 @@ describe Process do
214215
value.should eq("hello#{newline}")
215216
end
216217

217-
it "closes ios after block" do
218+
it "closes input after block" do
218219
Process.run(*stdin_to_stdout_command) { }
219220
$?.exit_code.should eq(0)
220221
end
221222

223+
it "closes output and error after block" do
224+
reader, writer = IO.pipe
225+
channel = Channel(Process).new
226+
227+
spawn do
228+
Process.run(*stdin_to_stdout_command, input: reader, output: :pipe, error: :pipe) do |process|
229+
channel.send process
230+
channel.receive
231+
end
232+
channel.close
233+
end
234+
235+
process = channel.receive
236+
237+
process.output.closed?.should be_false
238+
process.error.closed?.should be_false
239+
240+
channel.send process
241+
242+
# Wait a moment for the other fiber to continue and close the IOs
243+
wait_for { process.output.closed? }
244+
245+
process.output.closed?.should be_true
246+
process.error.closed?.should be_true
247+
248+
writer.close
249+
channel.receive?.should be_nil
250+
end
251+
222252
it "forwards closed io" do
223253
closed_io = IO::Memory.new
224254
closed_io.close

spec/support/wait_for.cr

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
def wait_for(timeout = 100.milliseconds, sleeping = 10.microseconds, &)
2+
now = Time.instant
3+
4+
Fiber.yield
5+
6+
until value = yield
7+
sleep sleeping
8+
9+
if now.elapsed > timeout
10+
return nil
11+
end
12+
13+
sleeping *= 2
14+
end
15+
value
16+
end

src/process.cr

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,8 @@ class Process
202202
process = new(command, args, env, clear_env, shell, input, output, error, chdir)
203203
begin
204204
value = yield process
205+
206+
process.close
205207
$? = process.wait
206208
value
207209
rescue ex
@@ -375,8 +377,6 @@ class Process
375377

376378
# Waits for this process to complete and closes any pipes.
377379
def wait : Process::Status
378-
close_io @input # only closed when a pipe was created but not managed by copy_io
379-
380380
@wait_count.times do
381381
ex = channel.receive
382382
raise ex if ex

0 commit comments

Comments
 (0)