Skip to content

Commit ca63974

Browse files
authored
Better handling of interrupts during stop/terminate. (#308)
1 parent a6a0091 commit ca63974

File tree

3 files changed

+53
-4
lines changed

3 files changed

+53
-4
lines changed

lib/async/scheduler.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ def scheduler_close
4949
self.close
5050
end
5151

52+
# Terminate the scheduler. We deliberately ignore interrupts here, as this code can be called from an interrupt, and we don't want to be interrupted while cleaning up.
53+
def terminate
54+
Thread.handle_interrupt(::Interrupt => :never) do
55+
super
56+
end
57+
end
58+
5259
# @public Since `stable-v1`.
5360
def close
5461
# It's critical to stop all tasks. Otherwise they might be holding on to resources which are never closed/released correctly.
@@ -308,7 +315,7 @@ def run(...)
308315

309316
begin
310317
# In theory, we could use Exception here to be a little bit safer, but we've only shown the case for SignalException to be a problem, so let's not over-engineer this.
311-
Thread.handle_interrupt(SignalException => :never) do
318+
Thread.handle_interrupt(::SignalException => :never) do
312319
while true
313320
# If we are interrupted, we need to exit:
314321
break if self.interrupted?
@@ -318,7 +325,10 @@ def run(...)
318325
end
319326
end
320327
rescue Interrupt
321-
self.stop
328+
Thread.handle_interrupt(::SignalException => :never) do
329+
self.stop
330+
end
331+
322332
retry
323333
end
324334

lib/async/task.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ def wait
209209
def stop(later = false)
210210
if self.stopped?
211211
# If we already stopped this task... don't try to stop it again:
212-
return
212+
return stopped!
213213
end
214214

215215
# If the fiber is alive, we need to stop it:
@@ -304,7 +304,7 @@ def stopped!
304304
stopped = false
305305

306306
begin
307-
# We are bnot running, but children might be so we should stop them:
307+
# We are not running, but children might be so we should stop them:
308308
stop_children(true)
309309
rescue Stop
310310
stopped = true

test/async/scheduler.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,45 @@
124124

125125
expect(finished).to be == true
126126
end
127+
128+
it "ignores interrupts during termination" do
129+
sleeping = Thread::Queue.new
130+
131+
thread = Thread.new do
132+
Thread.current.report_on_exception = false
133+
134+
scheduler = Async::Scheduler.new
135+
Fiber.set_scheduler(scheduler)
136+
137+
scheduler.run do |task|
138+
2.times do
139+
task.async do
140+
sleeping.push(true)
141+
sleep
142+
ensure
143+
sleeping.push(true)
144+
sleep
145+
end
146+
end
147+
end
148+
end
149+
150+
# The first interrupt stops the tasks normally, but they enter sleep again:
151+
expect(sleeping.pop).to be == true
152+
thread.raise(Interrupt)
153+
154+
# The second stop forcefully stops the two children tasks of the selector:
155+
expect(sleeping.pop).to be == true
156+
thread.raise(Interrupt)
157+
158+
# The thread should now exit:
159+
begin
160+
thread.join
161+
rescue Interrupt
162+
# Ignore - this may happen:
163+
# https://github.com/ruby/ruby/pull/10039
164+
end
165+
end
127166
end
128167

129168
with '#block' do

0 commit comments

Comments
 (0)