The example of skipping each controller in the README may be unstable

[astronoka]

skip_bullet may not be thread-safe. Sometimes false is restored when i expect it to be restored from false to true.

versions

ruby 2.7.1 bullet 6.1.0

test code

require 'bullet'

Bullet.enable = true
puts "# initial value: #{Bullet.enable?}"

$restore_logs = []

def skip_bullet
  previous_value = Bullet.enable?
  Bullet.enable = false
  yield
ensure
  # Expect a global interpreter lock for the implementation of `Array#<<`
  $restore_logs << "thread #{Thread.current} : restore Bullet.enable from #{Bullet.enable?} to #{previous_value}"
  Bullet.enable = previous_value
end

threads = 10.times.map do
  sleep(0.3)
  Thread.start do
    skip_bullet do
      # Time-consuming process
      sleep(1)
    end
  end
end

threads.each(&:join)

puts '# restore_logs'
$restore_logs.each do |log|
  puts " - #{log}"
end

puts "# last value: #{Bullet.enable?}"

output

# initial value: true
# restore_logs
 - thread #<Thread:0x00007fe966135d38 test.rb:22 run> : restore Bullet.enable from false to true
 - thread #<Thread:0x00007fe966135ba8 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe966135a90 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe966135928 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe9661356d0 test.rb:22 run> : restore Bullet.enable from false to true
 - thread #<Thread:0x00007fe9661354a0 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe966135248 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe966135018 test.rb:22 run> : restore Bullet.enable from false to false
 - thread #<Thread:0x00007fe966134dc0 test.rb:22 run> : restore Bullet.enable from false to true
 - thread #<Thread:0x00007fe966134b90 test.rb:22 run> : restore Bullet.enable from true to false
# last value: false

It's an unstable idiom. My teammate tried to use this code. How about deleting it from the README? If that's okay, I'll create a PR.

I'm not familiar with rails, so I'm sorry if I'm wrong about how to test it. Thanks for the nice module!

[michalwa]

Thank you for the finding and report @astronoka! Stumbled on the same problem. After some digging through Bullet's source, I was able to mitigate the problem by using the following helper method, instead of the one recommended in the README:

def pause_bullet
  previously_started = Bullet.start?
  Bullet.end_request if previously_started
  yield
ensure
  Bullet.start_request if previously_started
end

end_request and start_request seem to only mutate thread locals. I can't really prove this solves the problem completely, but the random failures sure seem to be reduced or gone.

It would be nice to get some assurance that this is indeed a good solution and if so, maybe update the README?

Side note: The problem seems to be more apparent as you increase RAILS_MAX_THREADS. Though I'm unsure of the exact conditions that need to occur.

Versions

Ruby: 3.2.2 Bullet: 7.0.7 Rails: 7.0.7.2