Skip to content

ThreadPool.shrink() miscounts pending shutdowns, and a stopped pool can hang on restartΒ #861

Open

Description

@rm-you

❓ I'm submitting a ...

  • 🐞 bug report
  • 🐣 feature request
  • ❓ question about the decisions made in the repository

🐞 Describe the bug. What is the current behavior?

Three related problems with how ThreadPool counts the shutdowns that shrink() queues:

  1. Calls close together are mostly ignored. shrink(amount) first subtracts every pending shutdown from amount, then removes the threads that have exited. A shutdown stays pending until a later call reaps its thread, so a call made after an earlier one is reduced by shutdowns that may already be done. Four shrink(20) calls in a row on 90 threads with a minimum of 10 leave 70 threads; a second apart, they leave 50.
  2. The pool can drop below min. The threads above the minimum are counted together with those already shutting down. shrink(40) then shrink(80) on 50 threads with a minimum of 10 retire all 50, and leave 30 shutdown requests queued.
  3. A stopped pool can hang on restart. stop() queues a shutdown request for every thread in _threads, including threads that shrink() already retired but that were not reaped yet, and never resets _pending_shutdowns. The requests left over stop the threads of the next start(), whose grow() then waits forever for them to be ready. HTTPServer.prepare() on a server that was stopped, as CherryPy does when its bus restarts, starts the pool again.

❓ What is the motivation / use case for changing the behavior?

Applications that size the pool to their load, as dynpool does from idle, or as oslo.messaging's HTTP-RPC driver does when RPC servers start and stop, end up with far more threads than they asked for, or none, and a restart that never returns.

#204 introduced _pending_shutdowns for cherrypy/cherrypy#1206, and its review settled that amount counts the shutdowns already scheduled. But idle already leaves them out, so a caller that sizes amount from idle has them subtracted twice, and the threads above the minimum still count them.

πŸ’‘ To Reproduce

import threading
import time

from cheroot import wsgi


def app(environ, start_response):
    start_response('200 OK', [('Content-Length', '0')])
    return [b'']


def pool_of(minthreads, grow):
    server = wsgi.Server(('127.0.0.1', 0), app, numthreads=minthreads)
    server.prepare()
    server.requests.grow(grow)
    return server, server.requests


def live(pool):
    return sum(t.is_alive() for t in pool._threads)


server, pool = pool_of(10, 80)  # 90 threads, at least 10
for _ in range(4):
    pool.shrink(20)
time.sleep(1)
print('4 x shrink(20) on 90 threads, min 10:', live(pool), 'left')
server.stop()

server, pool = pool_of(10, 40)  # 50 threads, at least 10
pool.shrink(40)
pool.shrink(80)
time.sleep(1)
print('shrink(40), shrink(80) on 50 threads, min 10:', live(pool), 'left,',
      pool.qsize, 'shutdown requests still queued')
server.stop()

server, pool = pool_of(2, 5)
pool.shrink(5)
time.sleep(1)
server.stop()
restart = threading.Thread(target=pool.start, daemon=True)
restart.start()
restart.join(10)
print('start() after shrink() and stop():',
      'still waiting after 10 s' if restart.is_alive() else 'returned')

With cheroot 11.1.2, and the same on main at 8402e55, where tox passes:

4 x shrink(20) on 90 threads, min 10: 70 left
shrink(40), shrink(80) on 50 threads, min 10: 0 left, 30 shutdown requests still queued
start() after shrink() and stop(): still waiting after 10 s

πŸ’‘ Expected behavior

4 x shrink(20) on 90 threads, min 10: 10 left
shrink(40), shrink(80) on 50 threads, min 10: 10 left, 0 shutdown requests still queued
start() after shrink() and stop(): returned

πŸ“‹ Details

cheroot/workers/threadpool.py on main (8402e55): shrink() at lines 344-353 (the subtraction before _clear_dead_threads(), and n_extra without the pending shutdowns), and stop() at lines 385-386 (a request per listed thread) and 423-431 (_clear_threads(), which leaves _pending_shutdowns as it is).

πŸ“‹ Environment

  • Cheroot version: 11.1.2, and main at 8402e55; the code is the same in 10.0.0
  • CherryPy version: not used
  • Python version: 3.11.2
  • OS: Linux (Debian 12)
  • Browser: not applicable

πŸ“‹ Additional context

A fix is proposed in #859: shrink() asks up to amount more threads to shut down, among those above the minimum that are not shutting down yet, and stop() queues one request per live thread that shrink() has not asked already, then forgets the pending shutdowns.

Related: the statistics retention reported in #860.


This report was written by an AI agent (Anthropic Claude Opus 5.5) on behalf of Adam Harwell (@rm-you). The reproductions were run against cheroot 11.1.2 from PyPI and a checkout of main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions