Skip to content

Commit

Permalink
Improve OOB hook
Browse files Browse the repository at this point in the history
This moves the hook to be executed outside of busy loop
considering the thread to be truly idle at a time of execution
of the hook
  • Loading branch information
ayufan committed Apr 23, 2020
1 parent b0b17f6 commit 92bae74
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 7 deletions.
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -38,6 +38,7 @@
* Fixed a few minor concurrency bugs in ThreadPool that may have affected non-GVL Rubies (#2220)
* Fix `out_of_band` hook never executed if the number of worker threads is > 1 (#2177)
* Fix ThreadPool#shutdown timeout accuracy (#2221)
* Call `out_of_band` hook outside of busy loop (#2230)

* Refactor
* Remove unused loader argument from Plugin initializer (#2095)
Expand Down
7 changes: 1 addition & 6 deletions lib/puma/server.rb
Expand Up @@ -226,6 +226,7 @@ def run(background=true)
end
end

@thread_pool.out_of_band_hook = @options[:out_of_band]
@thread_pool.clean_thread_locals = @options[:clean_thread_locals]

if @queue_requests
Expand Down Expand Up @@ -436,12 +437,6 @@ def process_client(client, buffer)
rescue StandardError => e
@events.unknown_error self, e, "Client"
end

if @options[:out_of_band]
@thread_pool.with_mutex do
@options[:out_of_band].each(&:call) if @thread_pool.busy_threads == 1
end
end
end
end

Expand Down
25 changes: 24 additions & 1 deletion lib/puma/thread_pool.rb
Expand Up @@ -65,6 +65,7 @@ def initialize(min, max, *extra, &block)

attr_reader :spawned, :trim_requested, :waiting
attr_accessor :clean_thread_locals
attr_accessor :out_of_band_hook

def self.clean_thread_locals
Thread.current.keys.each do |key| # rubocop: disable Performance/HashEachMethods
Expand Down Expand Up @@ -117,6 +118,7 @@ def spawn_thread

@waiting += 1
not_full.signal
trigger_out_of_band_hook
not_empty.wait mutex
@waiting -= 1
end
Expand All @@ -143,9 +145,30 @@ def spawn_thread

private :spawn_thread

def trigger_out_of_band_hook
return unless out_of_band_hook && out_of_band_hook.any?

with_mutex do |mutex|
# we execute on idle hook when all threads are free
return unless @spawned == @waiting

# we unlock thread for the duration of out of band hook
# to allow other requests to be accepted
mutex.unlock

begin
out_of_band_hook.each(&:call)
rescue Exception => e
STDERR.puts "Exception calling out_of_band_hook: #{e.message} (#{e.class})"
ensure
mutex.lock
end
end
end

def with_mutex(&block)
@mutex.owned? ?
yield :
yield(@mutex) :
@mutex.synchronize(&block)
end

Expand Down

0 comments on commit 92bae74

Please sign in to comment.