Opened 5 years ago

Closed 5 years ago

#7011 defect closed fixed (fixed)

Adjusting the size of a stopped thread pool starts new threads.

Reported by: Tom Prince Owned by: hawkowl
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch: branches/threadpool-adjust-hang-7011
branch-diff, diff-cov, branch-cov, buildbot
Author: hawkowl


The following code will hang:

from twisted.python.threadpool import ThreadPool
pool = ThreadPool()

Change History (5)

comment:1 Changed 5 years ago by hawkowl

Author: hawkowl
Branch: branches/threadpool-adjust-hang-7011

(In [41781]) Branching to threadpool-adjust-hang-7011.

comment:2 Changed 5 years ago by hawkowl

Keywords: review added
Type: enhancementdefect

The pool didn't mark itself as not started when it was stopped - I think this is an oversight? Unless it means it was never started... anyway, I've added that and a test for that case. It hangs as tomprince says if I run it on master.

Putting up for review.

comment:3 Changed 5 years ago by hawkowl

Summary: Adjusting the size of a stopped thead pool starts new threads.Adjusting the size of a stopped thread pool starts new threads.

comment:4 Changed 5 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to hawkowl

Thanks. Just a few minor comments:

  1. Making sure that a stopped pool doesn't start new workers when the size is adjusted. isn't a proper test method docstring.
  2. The ThreadPool.threads attribute is undocumented but the new test relies on it having a particular meaning. Please document that meaning. I think the expected behavior is already tested by test_start but if you wanted to add a more direct test that would be cool too.
  3. ThreadPool.started is also undocumented. Can you document it?
  4. In the news fragment I would say "but the pool is stopped" rather than "and the pool is stopped". Alternatively, "and the pool is not running." "stopped" could indicate a state or an action.

Please merge after addressing those points. Thanks again.

comment:5 Changed 5 years ago by hawkowl

Resolution: fixed
Status: newclosed

(In [41786]) Merge threadpool-adjust-hang-7011

Author: hawkowl Reviewer: exarkun Fixes: #7011

twisted.python.threadpool.ThreadPool no longer starts new workers when its pool size is changed while the pool is not running.

Note: See TracTickets for help on using tickets.