Skip to content

Commit

Permalink
Use taskkill to kill a process tree on Windows
Browse files Browse the repository at this point in the history
* Inspired by https://stackoverflow.com/a/61113184/388803
* Using a subprocess for that is a bit heavy, OTOH we are already creating
  subprocess in ChildProcess and it only happens for a leader process on Windows.
* There seems to be no other alternative which works reliably or it would incur huge complexity.
  • Loading branch information
eregon committed Jan 4, 2024
1 parent 228ec6d commit 17ee66b
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 55 deletions.
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,6 @@ process.leader = true
process.start
```

Note this does not work on Windows.

#### Detach from parent

```ruby
Expand Down
4 changes: 1 addition & 3 deletions lib/childprocess/abstract_process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ class AbstractProcess
# Set this to true to make the child process the leader of a new process group
#
# This can be used to make sure that all grandchildren are killed
# when the child process dies on non-Windows.
#
# Note that grandchildren are not killed on Windows even when the process is the leader of a new process group.
# when the child process dies.
#
attr_accessor :leader

Expand Down
12 changes: 4 additions & 8 deletions lib/childprocess/process_spawn_process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,19 +112,15 @@ def send_signal(sig)
assert_started

log "sending #{sig}"
::Process.kill sig, _pid
end

def _pid
if leader?
if ChildProcess.unix?
-@pid # negative pid == process group
::Process.kill sig, -@pid # negative pid == process group
else
ChildProcess.logger.warn "ChildProcess#stop on leader of a new process group does not kill subprocess on Windows"
@pid
output = `taskkill /F /T /PID #{@pid}`
log output
end
else
@pid
::Process.kill sig, @pid
end
end
end
Expand Down
52 changes: 10 additions & 42 deletions spec/childprocess_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -299,52 +299,20 @@
}
end

if ChildProcess.unix?
it 'kills the full process tree on unix' do
Tempfile.open('kill-process-tree') do |file|
process = write_pid_in_sleepy_grand_child(file.path)
process.leader = true
process.start

pid = wait_until(30) do
Integer(rewind_and_read(file)) rescue nil
end

process.stop
expect(alive?(process.pid)).to eql(false)
it 'kills the full process tree on unix' do
Tempfile.open('kill-process-tree') do |file|
process = write_pid_in_sleepy_grand_child(file.path)
process.leader = true
process.start

wait_until(3) { expect(alive?(pid)).to eql(false) }
pid = wait_until(30) do
Integer(rewind_and_read(file)) rescue nil
end
end
elsif ChildProcess.windows?
it 'does not kill the full process tree on windows' do
Tempfile.open('no-kill-process-tree') do |file|
process = write_pid_in_sleepy_grand_child(file.path)
process.leader = true
process.start

pid = wait_until(30) do
Integer(rewind_and_read(file)) rescue nil
end
process.stop
expect(process).to be_exited

log = StringIO.new
original_logger = ChildProcess.logger
begin
ChildProcess.logger = Logger.new(log)
ChildProcess.logger.level = Logger::WARN
process.stop
ensure
ChildProcess.logger = original_logger
end
expect(log.string).to include("ChildProcess#stop on leader of a new process group does not kill subprocess on Windows")

expect(alive?(process.pid)).to eql(false)

# The grand child is not killed on Windows:
expect(alive?(pid)).to eql(true)
Process.kill(:SIGKILL, pid)
wait_until(3) { expect(alive?(pid)).to eql(false) }
end
wait_until(3) { expect(alive?(pid)).to eql(false) }
end
end

Expand Down

0 comments on commit 17ee66b

Please sign in to comment.