Jens Geyer created THRIFT-6082:
----------------------------------
Summary: Python TProcessPoolServer test shutdown can deadlock
(signal handler reenters Condition.notify())
Key: THRIFT-6082
URL: https://issues.apache.org/jira/browse/THRIFT-6082
Project: Thrift
Issue Type: Bug
Components: Python - Library
Reporter: Jens Geyer
h3. Summary
{{test/py/TestServer.py}}'s SIGALRM-based test shutdown can deadlock the server
subprocess indefinitely. This surfaced as a {{lib-python-macos}} CI job that
ran until GitHub Actions' 6-hour job ceiling before being cancelled, even
though all 24 client-side test assertions had already passed.
h3. Root cause
{{TestServer.py}} runs the {{TProcessPoolServer}} server flavor under a
{{signal.alarm(4)}} self-timer. When the alarm fires, the handler
({{clean_shutdown}}) terminates the worker processes and calls
{{server.stop()}} ({{lib/py/src/server/TProcessPoolServer.py}}).
{{TProcessPoolServer.serve()}} blocks its main thread in
{{multiprocessing.Condition.wait()}} while waiting to be told to stop. Python
signal handlers run synchronously, nested inside whatever the interrupted
thread was doing -- so {{clean_shutdown}} reenters on the *same* OS thread that
is parked inside that {{wait()}} call. {{stop()}}'s call to
{{Condition.notify()}} then blocks waiting for the sleeper to post a wake
acknowledgement (the {{_woken_count}} handshake in
{{multiprocessing/synchronize.py}}) -- but the "sleeper" is the very thread now
trapped inside the nested signal handler, so it can never reach that code. The
two frames wait on each other forever.
This reproduces deterministically (10/10 runs, both {{fork}} and {{spawn}}
multiprocessing start methods) against the unmodified library code, confirmed
with a {{faulthandler}} stack trace showing the frozen frame inside
{{Condition.notify()}}.
h3. Impact
* In CI: a single {{TProcessPoolServer}} test iteration hangs, and nothing else
in that job runs afterward.
* In production: any application that calls {{TProcessPoolServer.stop()}} from
a signal handler (a common graceful-shutdown pattern, e.g. on SIGTERM/SIGINT)
can hit the identical deadlock if the signal happens to arrive while the
server's main thread is parked in {{wait()}}.
h3. Fix
{{test/py/TestServer.py}}'s shutdown handler should terminate the workers and
exit the process directly instead of coordinating through
{{TProcessPoolServer.stop()}}'s {{Condition}}, avoiding the reentrant
{{notify()}} call. PR to follow.
h3. Out of scope
The underlying signal-handler/{{Condition}} reentrancy hazard inside
{{TProcessPoolServer}} itself (library-level, not just the test harness) is not
addressed by the fix above and would need separate design work (e.g. a
signal-safe polling loop instead of {{Condition.wait()}}) to harden it for
production callers.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
