Skip to content

Commit

Permalink
Fix race condition starting CacheCleanupThread (#586)
Browse files Browse the repository at this point in the history
This fixes two race conditions

1. Thread execution may start before `CacheCleanupThread.new` returns the class instance. In this case `t` is not yet defined inside the thread block and results in a "undefined method `sleepy_run' for nil:NilClass" error.

2. Thread execution may start before member variables `@store`, `@interval`, etc are initialized in `CacheCleanupThread#initialize` This results in "undefined method '*' for nil:NilClass" in should_cleanup? (this error only occurs after the first race condition is fixed)

More detail: A subclassed thread is scheduled for execution as soon as `super` is called. The code block of the thread may start to execute before the class is fully instantiated. See https://www.ruby-forum.com/t/thread-super-should-be-first-line-or-last-line/150617
  • Loading branch information
willkoehler committed Jun 22, 2023
1 parent b6cf1b5 commit 0d34c02
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/mini_profiler/storage/memory_store.rb
Expand Up @@ -10,11 +10,11 @@ class MemoryStore < AbstractStore
class CacheCleanupThread < Thread

def initialize(interval, cycle, store)
super
@store = store
@interval = interval
@cycle = cycle
@cycle_count = 1
super
end

def should_cleanup?
Expand Down Expand Up @@ -78,7 +78,7 @@ def initialize_cleanup_thread(args = {})
cleanup_cycle = args.fetch(:cleanup_cycle) { CLEANUP_CYCLE }
t = CacheCleanupThread.new(cleanup_interval, cleanup_cycle, self) do
until Thread.current[:should_exit] do
t.sleepy_run
Thread.current.sleepy_run
end
end
at_exit { t[:should_exit] = true }
Expand Down

0 comments on commit 0d34c02

Please sign in to comment.