[
https://issues.apache.org/jira/browse/THRIFT-6082?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer reassigned THRIFT-6082:
----------------------------------
Assignee: Jens Geyer
> 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
> Assignee: Jens Geyer
> Priority: Major
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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)
