Skip to content

Commit 0e653a7

Browse files
committed
Robust STDIN pass-through to sub-process.
My previous attempt at preventing explosive thread growth didn't work because I couldn't reliably interrupt a .gets call. The timeout in the thread did cause the thread to begin aborting, but would still be blocked. Starting multiple such threads meant any user input could end up being consumed by any one of those threads. Moving towards a single thread for getting user input and communicating to a new pass-through thread via a Queue avoids those problems and allows us to stop the pass-through threads when they're no longer needed.
1 parent 925f4bc commit 0e653a7

File tree

1 file changed

+26
-5
lines changed

1 file changed

+26
-5
lines changed

lib/svn2git/migration.rb

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,15 @@ def run_command(cmd, exit_on_error=true, printout_output=false)
342342
log "Running command: #{cmd}"
343343

344344
ret = ''
345-
mutex = Mutex.new
345+
@mutex ||= Mutex.new
346+
@stdin_queue ||= Queue.new
347+
348+
# We need to fetch input from the user to pass through to the underlying sub-process. We'll constantly listen
349+
# for input and place any received values on a queue for consumption by a pass-through thread that will forward
350+
# the contents to the underlying sub-process's stdin pipe.
351+
@stdin_thread ||= Thread.new do
352+
loop { @stdin_queue << $stdin.gets.chomp }
353+
end
346354

347355
# Open4 forks, which JRuby doesn't support. But JRuby added a popen4-compatible method on the IO class,
348356
# so we can use that instead.
@@ -351,7 +359,7 @@ def run_command(cmd, exit_on_error=true, printout_output=false)
351359

352360
threads << Thread.new(stdout) do |stdout|
353361
stdout.each do |line|
354-
mutex.synchronize do
362+
@mutex.synchronize do
355363
ret << line
356364

357365
if printout_output
@@ -364,8 +372,12 @@ def run_command(cmd, exit_on_error=true, printout_output=false)
364372
end
365373

366374
threads << Thread.new(stderr) do |stderr|
375+
# git-svn seems to do all of its prompting for user input via STDERR. When it prompts for input, it will
376+
# not terminate the line with a newline character, so we can't split the input up by newline. It will,
377+
# however, use a space to separate the user input from the prompt. So we split on word boundaries here
378+
# while draining STDERR.
367379
stderr.each(' ') do |word|
368-
mutex.synchronize do
380+
@mutex.synchronize do
369381
ret << word
370382

371383
if printout_output
@@ -377,15 +389,24 @@ def run_command(cmd, exit_on_error=true, printout_output=false)
377389
end
378390
end
379391

392+
# Simple pass-through thread to take anything the user types via STDIN and passes it through to the
393+
# sub-process's stdin pipe.
380394
Thread.new(stdin) do |stdin|
381-
until $stdin.closed?
382-
user_reply = Timeout.timeout(5) { $stdin.gets.chomp }
395+
loop do
396+
user_reply = @stdin_queue.pop
397+
398+
# nil is our cue to stop looping (pun intended).
399+
break if user_reply.nil?
400+
383401
stdin.puts user_reply
384402
stdin.close
385403
end
386404
end
387405

388406
threads.each(&:join)
407+
408+
# Push nil to the stdin_queue to gracefully exit the STDIN pass-through thread.
409+
@stdin_queue << nil
389410
end
390411

391412
# JRuby's open4 doesn't return a Process::Status object when invoked with a block, but rather returns the

0 commit comments

Comments
 (0)