Opened 7 months ago

Closed 7 months 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
(diff, github, buildbot, log)
Author: hawkowl Launchpad Bug:

Description

The following code will hang:

from twisted.python.threadpool import ThreadPool
pool = ThreadPool()
pool.start()
pool.stop()
pool.adjustPoolsize()

Change History (5)

comment:1 Changed 7 months ago by hawkowl

  • Author set to hawkowl
  • Branch set to branches/threadpool-adjust-hang-7011

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

comment:2 Changed 7 months ago by hawkowl

  • Keywords review added
  • Type changed from enhancement to defect

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 7 months ago by hawkowl

  • Summary changed from Adjusting the size of a stopped thead pool starts new threads. to Adjusting the size of a stopped thread pool starts new threads.

comment:4 Changed 7 months ago by exarkun

  • 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 7 months ago by hawkowl

  • Resolution set to fixed
  • Status changed from new to closed

(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.