Skip to content

Commit c67867f

Browse files
committed
Use SIGKILL/Thread#kill when the health check fails.
1 parent d9f8c33 commit c67867f

File tree

5 files changed

+42
-4
lines changed

5 files changed

+42
-4
lines changed

fixtures/async/container/a_container.rb

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,23 @@ module Container
201201
instance.ready!
202202

203203
# This should trigger the health check - since restart is false, the process will be terminated:
204-
sleep(2.0)
204+
sleep
205+
end
206+
207+
container.wait
208+
209+
expect(container.statistics).to have_attributes(failures: be > 0)
210+
end
211+
212+
it "can kill a child process even if it ignores exceptions/signals" do
213+
container.spawn(health_check_timeout: 1.0) do |instance|
214+
while true
215+
begin
216+
sleep 1
217+
rescue Exception => error
218+
# Ignore.
219+
end
220+
end
205221
end
206222

207223
container.wait

lib/async/container/forked.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,13 @@ def terminate!
180180
end
181181
end
182182

183+
# Send `SIGKILL` to the child process.
184+
def kill!
185+
unless @status
186+
::Process.kill(:KILL, @pid)
187+
end
188+
end
189+
183190
# Send `SIGHUP` to the child process.
184191
def restart!
185192
unless @status

lib/async/container/generic.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,8 +172,8 @@ def spawn(name: nil, restart: false, key: nil, health_check_timeout: nil, &block
172172
when :health_check!
173173
if health_check_timeout&.<(age_clock.total)
174174
Console.warn(self, "Child failed health check!", child: child, age: age_clock.total, health_check_timeout: health_check_timeout)
175-
# If the child has failed the health check, we assume the worst and terminate it (SIGTERM).
176-
child.terminate!
175+
# If the child has failed the health check, we assume the worst and kill it immediately:
176+
child.kill!
177177
end
178178
else
179179
state.update(message)

lib/async/container/threaded.rb

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ module Async
1111
module Container
1212
# A multi-thread container which uses {Thread.fork}.
1313
class Threaded < Generic
14+
class Kill < Exception
15+
end
16+
1417
# Indicates that this is not a multi-process container.
1518
def self.multiprocess?
1619
false
@@ -178,6 +181,14 @@ def terminate!
178181
@thread.raise(Terminate)
179182
end
180183

184+
# Invoke {Thread#kill} on the child thread.
185+
def kill!
186+
# Killing a thread does not raise an exception in the thread, so we need to handle the status here:
187+
@status = Status.new(:killed)
188+
189+
@thread.kill
190+
end
191+
181192
# Raise {Restart} in the child thread.
182193
def restart!
183194
@thread.raise(Restart)
@@ -230,7 +241,7 @@ def finished(error = nil)
230241
Console.error(self) {error}
231242
end
232243

233-
@status = Status.new(error)
244+
@status ||= Status.new(error)
234245
self.close_write
235246
end
236247
end

releases.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Releases
22

3+
## Unreleased
4+
5+
- Use `SIGKILL`/`Thread#kill` when the health check fails. In some cases, `SIGTERM` may not be sufficient to terminate a process because the signal can be ignored or the process may be in an uninterruptible state.
6+
37
## v0.20.1
48

59
- Fix compatibility between {ruby Async::Container::Hybrid} and the health check.

0 commit comments

Comments
 (0)