From 29f7ec709068dd87375afec1b61fe46cd37c48b9 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Sun, 15 Jan 2023 22:08:44 +0000 Subject: [PATCH] DEV: Prevent defer stats exception when thread aborted (#19863) When the thread is aborted, an exception is raised before the `start` of a job is set, and therefore raises an exception in the `ensure` block. This commit checks that `start` exists, and also adds `abort_on_exception=true` so that this issue would have caused test failures. --- lib/discourse.rb | 14 +++++++++++++- lib/scheduler/defer.rb | 18 ++++++++++++------ spec/lib/scheduler/defer_spec.rb | 6 +++++- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/lib/discourse.rb b/lib/discourse.rb index 577ae781008..0fa1598d3cc 100644 --- a/lib/discourse.rb +++ b/lib/discourse.rb @@ -191,6 +191,18 @@ module Discourse reset_job_exception_stats! + if Rails.env.test? + def self.catch_job_exceptions! + raise "tests only" if !Rails.env.test? + @catch_job_exceptions = true + end + + def self.reset_catch_job_exceptions! + raise "tests only" if !Rails.env.test? + remove_instance_variable(:@catch_job_exceptions) + end + end + # Log an exception. # # If your code is in a scheduled job, it is recommended to use the @@ -220,7 +232,7 @@ module Discourse { current_db: cm.current_db, current_hostname: cm.current_hostname }.merge(context), ) - raise ex if Rails.env.test? + raise ex if Rails.env.test? && !@catch_job_exceptions end # Expected less matches than what we got in a find diff --git a/lib/scheduler/defer.rb b/lib/scheduler/defer.rb index 6b059280060..176ae1a62a3 100644 --- a/lib/scheduler/defer.rb +++ b/lib/scheduler/defer.rb @@ -79,7 +79,11 @@ module Scheduler def start_thread @mutex.synchronize do @reactor = MessageBus::TimerThread.new if !@reactor - @thread = Thread.new { do_work while true } if !@thread&.alive? + @thread = + Thread.new do + @thread.abort_on_exception = true if Rails.env.test? + do_work while true + end if !@thread&.alive? end end @@ -110,11 +114,13 @@ module Scheduler Discourse.handle_job_exception(ex, message: "Processing deferred code queue") ensure ActiveRecord::Base.connection_handler.clear_active_connections! - @stats_mutex.synchronize do - stats = @stats[desc] - if stats - stats[:finished] += 1 - stats[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start + if start + @stats_mutex.synchronize do + stats = @stats[desc] + if stats + stats[:finished] += 1 + stats[:duration] += Process.clock_gettime(Process::CLOCK_MONOTONIC) - start + end end end end diff --git a/spec/lib/scheduler/defer_spec.rb b/spec/lib/scheduler/defer_spec.rb index 266af82bc39..f886335fd2a 100644 --- a/spec/lib/scheduler/defer_spec.rb +++ b/spec/lib/scheduler/defer_spec.rb @@ -12,11 +12,15 @@ RSpec.describe Scheduler::Defer do end before do + Discourse.catch_job_exceptions! @defer = DeferInstance.new @defer.async = true end - after { @defer.stop! } + after do + @defer.stop! + Discourse.reset_catch_job_exceptions! + end it "supports basic instrumentation" do @defer.later("first") {}