search

MiniProfiler / rack-mini-profiler

Profiler for your development and production Ruby rack apps.
MIT License
3.68k stars 400 forks source link

Fix race condition starting CacheCleanupThread #586

Closed willkoehler closed 1 year ago

willkoehler commented 1 year ago

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

Fixes #566

nateberkopec commented 1 year ago

Amazing work. There's not way I would have ever figured out point #2 especially... great job!

willkoehler commented 1 year ago

Thanks @nateberkopec 🙌 This was a particularly fun one. I had a hunch about points #1 and #2. But I applied them separately at first, which made it appear that neither one made a difference. It took a while to figure out they both needed to be applied.