Skip to content

Commit

Permalink
Minor refactor on Thread pool (#1088)
Browse files Browse the repository at this point in the history
* Move shutdown grace time constant to ThreadPool.
SHUTDOWN_GRACE_TIME is the only constant (from const.rb) used by
ThreadPool. It's better to move the constant than require all const.rb.
* Fix minor typo.

* Don't need to check if timeout is zero to immediately shutdown.
This removes the duplicated code and add test for forced shutdowns.
  • Loading branch information
frodsan authored and nateberkopec committed Nov 20, 2016
1 parent 13ac97f commit 64f930d
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 17 deletions.
5 changes: 0 additions & 5 deletions lib/puma/const.rb
Expand Up @@ -113,11 +113,6 @@ module Const
# sending data back
WRITE_TIMEOUT = 10

# How long, after raising the ForceShutdown of a thread during
# forced shutdown mode, to wait for the thread to try and finish
# up it's work before leaving the thread to die on the vine.
SHUTDOWN_GRACE_TIME = 5 # seconds

# The original URI requested by the client.
REQUEST_URI= 'REQUEST_URI'.freeze
REQUEST_PATH = 'REQUEST_PATH'.freeze
Expand Down
22 changes: 10 additions & 12 deletions lib/puma/thread_pool.rb
Expand Up @@ -4,10 +4,14 @@ module Puma
# A simple thread pool management object.
#
class ThreadPool

class ForceShutdown < RuntimeError
end

# How long, after raising the ForceShutdown of a thread during
# forced shutdown mode, to wait for the thread to try and finish
# up its work before leaving the thread to die on the vine.
SHUTDOWN_GRACE_TIME = 5 # seconds

# Maintain a minimum of +min+ and maximum of +max+ threads
# in the pool.
#
Expand Down Expand Up @@ -259,18 +263,12 @@ def shutdown(timeout=-1)
@workers.dup
end

case timeout
when -1
if timeout == -1
# Wait for threads to finish without force shutdown.
threads.each(&:join)
when 0
threads.each do |t|
t.raise ForceShutdown
end

threads.each do |t|
t.join Const::SHUTDOWN_GRACE_TIME
end
else
# Wait for threads to finish after n attempts (+timeout+).
# If threads are still running, it will forcefully kill them.
timeout.times do
threads.delete_if do |t|
t.join 1
Expand All @@ -288,7 +286,7 @@ def shutdown(timeout=-1)
end

threads.each do |t|
t.join Const::SHUTDOWN_GRACE_TIME
t.join SHUTDOWN_GRACE_TIME
end
end

Expand Down
17 changes: 17 additions & 0 deletions test/test_thread_pool.rb
Expand Up @@ -230,4 +230,21 @@ def test_auto_reap_dead_threads

assert_equal 0, pool.spawned
end

def test_force_shutdown_immediately
pool = new_pool(1, 1) {
begin
sleep 1
rescue Puma::ThreadPool::ForceShutdown
end
}

pool << 1

sleep 0.1

pool.shutdown(0)

assert_equal 0, pool.spawned
end
end

0 comments on commit 64f930d

Please sign in to comment.