Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better handling of interrupts during stop/terminate. #308

Merged
merged 1 commit into from
Feb 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib/async/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ def scheduler_close
self.close
end

# 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.
def terminate
Thread.handle_interrupt(::Interrupt => :never) do
super
end
end

# @public Since `stable-v1`.
def close
# It's critical to stop all tasks. Otherwise they might be holding on to resources which are never closed/released correctly.
Expand Down Expand Up @@ -308,7 +315,7 @@ def run(...)

begin
# 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.
Thread.handle_interrupt(SignalException => :never) do
Thread.handle_interrupt(::SignalException => :never) do
while true
# If we are interrupted, we need to exit:
break if self.interrupted?
Expand All @@ -318,7 +325,10 @@ def run(...)
end
end
rescue Interrupt
self.stop
Thread.handle_interrupt(::SignalException => :never) do
self.stop
end

retry
end

Expand Down
4 changes: 2 additions & 2 deletions lib/async/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ def wait
def stop(later = false)
if self.stopped?
# If we already stopped this task... don't try to stop it again:
return
return stopped!
end

# If the fiber is alive, we need to stop it:
Expand Down Expand Up @@ -304,7 +304,7 @@ def stopped!
stopped = false

begin
# We are bnot running, but children might be so we should stop them:
# We are not running, but children might be so we should stop them:
stop_children(true)
rescue Stop
stopped = true
Expand Down
39 changes: 39 additions & 0 deletions test/async/scheduler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,45 @@

expect(finished).to be == true
end

it "ignores interrupts during termination" do
sleeping = Thread::Queue.new

thread = Thread.new do
Thread.current.report_on_exception = false

scheduler = Async::Scheduler.new
Fiber.set_scheduler(scheduler)

scheduler.run do |task|
2.times do
task.async do
sleeping.push(true)
sleep
ensure
sleeping.push(true)
sleep
end
end
end
end

# The first interrupt stops the tasks normally, but they enter sleep again:
expect(sleeping.pop).to be == true
thread.raise(Interrupt)

# The second stop forcefully stops the two children tasks of the selector:
expect(sleeping.pop).to be == true
thread.raise(Interrupt)

# The thread should now exit:
begin
thread.join
rescue Interrupt
# Ignore - this may happen:
# https://github.com/ruby/ruby/pull/10039
end
end
end

with '#block' do
Expand Down
Loading