Repository navigation
test_concurrent_futures.test_shutdown: test_interpreter_shutdown() fails randomly (race condition) #109047
Description
The test_interpreter_shutdown() test of test_concurrent_futures.test_shutdown has a race condition. On purpose, the test doesn't wait until the executor completes (!). Moreover, it expects the executor to always be able to submit its job, and the job to complete successfully! It's a very optimistic bet.
See also issue #107219: test_concurrent_futures: test_crash_big_data() hangs randomly on Windows.
When Python is shutting down, Py_Finalize() quickly blocks the creation of new threads in _thread.start_new_thread():
cpython/Modules/_threadmodule.c
Lines 1161 to 1165 in 3bfa24e
| if (interp->finalizing) { | |
| PyErr_SetString(PyExc_RuntimeError, | |
| "can't create new thread at interpreter shutdown"); | |
| return NULL; | |
| } |
This exception was added recently (last June) by commit ce558e6: see issue gh-104690 for the rationale.
The multiprocessing executor spawns _ExecutorManagerThread thread which runs its "main loop" in its run() method:
cpython/Lib/concurrent/futures/process.py
Lines 335 to 378 in 3bfa24e
| def run(self): | |
| # Main loop for the executor manager thread. | |
| while True: | |
| self.add_call_item_to_queue() | |
| result_item, is_broken, cause = self.wait_result_broken_or_wakeup() | |
| if is_broken: | |
| self.terminate_broken(cause) | |
| return | |
| if result_item is not None: | |
| self.process_result_item(result_item) | |
| process_exited = result_item.exit_pid is not None | |
| if process_exited: | |
| p = self.processes.pop(result_item.exit_pid) | |
| p.join() | |
| # Delete reference to result_item to avoid keeping references | |
| # while waiting on new results. | |
| del result_item | |
| if executor := self.executor_reference(): | |
| if process_exited: | |
| with self.shutdown_lock: | |
| executor._adjust_process_count() | |
| else: | |
| executor._idle_worker_semaphore.release() | |
| del executor | |
| if self.is_shutting_down(): | |
| self.flag_executor_shutting_down() | |
| # When only canceled futures remain in pending_work_items, our | |
| # next call to wait_result_broken_or_wakeup would hang forever. | |
| # This makes sure we have some running futures or none at all. | |
| self.add_call_item_to_queue() | |
| # Since no new work items can be added, it is safe to shutdown | |
| # this thread if there are no pending work items. | |
| if not self.pending_work_items: | |
| self.join_executor_internals() | |
| return |
It tries to submit new jobs to the worker process through a queue, but oops, the Python main thread is finalizing (called Py_Finalizing())! There is not notification system to notify threads that Python is being finalized.
Moreover, there are 3 "finalization" states:
interp->finalizing-- used by_thread.start_new_thread()to block thread creation during Python finazlizationruntime->_finalizing-- used bysys.is_finalizing(),Py_IsFinalizing()and_PyRuntimeState_GetFinalizing(runtime)interp->_finalizing-- used byceval.cto decide if a Python thread "must exit" or not, as soon as it's set, all Python threads must exit as soon as they attempt to acquire the GIL
These 3 states at not set at the same time.
- Calling
Py_Finalize()setsinterp->finalizingto 1 as soon as possible: so spawning new threads is immediately blocked (which is a good thing to get a reliable finalization!) Py_Finalize()callsthreading._shutdown()which blocks until all non-daemon threads completesPy_Finalize()callsatexitcallbacks- And only then,
Py_Finalize()setsruntime->_finalizingandinterp->_finalizingto the Python thread state (tstate) which callsPy_Finalize()
The delay between (1) and (4) can be quite long, a thread can take several milliseconds, if not seconds, to complete.
Can multiprocessing or concurrent.futures check if Python is finalizing or be notified? Well, did you hear about Time-of-check to time-of-use race conditions? Even if it would be possible, I don't think that we can "check" if it's safe to spawn a thread just before spawning a thread, since the main thread can decide to finalize Python "at any time". It will become even more tricky with Python nogil ;-)
So what's left? Well, multiprocessing and concurrent.futures should be optimistic, call Python functions and only then check for exceptions. Depending on the exceptions, they can decide how to handle it. I would suggest to exit as soon as possible, and try to cleanup resources if possible.
Another option would be to make multiprocessing and concurrent.futures more determistic. Rather than spawning threads and processes in the background "on demand" and hope that everything will be fine, add more synchronization to "wait" until everything is ready to submit jobs. I think that I already tried this approach in the past, but @pitrou didn't like it since it made some workloads slower. You may not always need to actually submits jobs. You may not always need all threads and processes.
Well, I don't know even these complex modules to tell which option is the least bad :-)
Finally, as usually, I beg you to make these APIs less magical, and enforce more explicit resources management! It shouldn't even be possible to not wait until an executor complete. It should be enforced by emitting loudly ResourceWarning warnings :-) Well, that's my opinion. I know that it's not shared by @pitrou :-)
Linked PRs
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
vstinner commentedon Sep 7, 2023
This issue is made of multiple sub-issues.
Here is a script to reproduce the
RuntimeError: can't create new thread at interpreter shutdownerror:The script is based on
test_interpreter_shutdown()ofLib/test/test_concurrent_futures/test_shutdown.py.To trigger the issue, I run these two commands in parallel in two terminals:
while true; do ./python script.py || break; done./python -m test -j4 -r-- Increase-j4if you cannot reproduce the issue. In fact, any command to stress t the system works.I easily reproduce the issue on FreeBSD. So far I failed reproduce the bug with this method on Linux.
Logs:
vstinner commentedon Sep 7, 2023
I saw two kinds of behavior:
RuntimeError: can't create new thread at interpreter shutdownon Linux on FreeBSDErrors:
August 25: Windows: Azure Pipelines: "Windows PR Tests win32"
test_interpreter_shutdown()hangs: killed byTimeout (0:20:00)!August 26: Linux: "aarch64 Fedora Stable LTO + PGO 3.x" buildbot
test_interpreter_shutdown()ofProcessPoolForkserverProcessPoolShutdownTestRuntimeError: can\'t create new thread at interpreter shutdownAugust 26: Linux: "ARM Raspbian 3.x" buildbot
test_interpreter_shutdown()ofProcessPoolForkserverProcessPoolShutdownTestRuntimeError: can't create new thread at interpreter shutdownvstinner commentedon Sep 7, 2023
I confirm that the commit ce558e6 introduced the regression.
gpshead commentedon Sep 7, 2023
I'm not surprised about the "cause" commit that exposed these problems. I agree with your overall analysis theme: We need to figure out how to make this situation more clear and handle the errors appropriately so that things that blindly thought they could keep running forever get a concrete exception that they can catch and cleanup and shutdown gracefully with. In some cases this probably also means adjusting the expectation of some tests.
(I have noticed a few CI runs and buildbots with these errors popping up)
vstinner commentedon Sep 22, 2023
Trying to check if Python is being finalized before running an action is unsafe, there is still a risk that Python finalization starts just after the check, and so the action (like creating a thread) will fail.
The safe pattern is to raise an exception and catch the exception. My problem here is that RuntimeError is quite generic.
@ericsnowcurrently @gpshead: What would you think of introducing a new PythonFinalizationError exception uses by the few spots which blocks actions during Python finalization? I'm open to other name suggestions :-)
multiprocessing, threading and other modules could expect such error and cleans everything properly to handle Python finalization smoothly. For example, if a multiprocessing manager gets it, it would be a signal that ah, it's now time to stop everything.
multiprocessing already has special code path for BrokenPipeError which is treated in similar way: see commit a9b1f84 which raises BrokenPipeError if a pipe is closed, so the caller gets BrokenPipeError and can exit properly. See gh-107219 and PR #109244.
gpshead commentedon Sep 23, 2023
PythonFinalizationError is a good a name as any. This is the kind of exception we don't expect anyone (well, users anyways) to ever catch as there really isn't anything they can likely meaningfully do if the did. (like SystemError?)
pythongh-109047: Add PythonFinalizationError exception
pythongh-109047: Add PythonFinalizationError exception
pythongh-109047: Add PythonFinalizationError exception
pythongh-109047: concurrent.futures _ExecutorManagerThread catches Py…
pythongh-109047: concurrent.futures catches PythonFinalizationError
pythongh-109047: concurrent.futures catches PythonFinalizationError
pythongh-109047: concurrent.futures catches PythonFinalizationError
11 remaining items
[3.12] gh-109047: concurrent.futures catches RuntimeError (#109810) (#…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] gh-114440: Close writer pipe in multiprocessing.Queue, not con…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…
pythongh-109047: concurrent.futures catches PythonFinalizationError (p…
[3.12] pythongh-114440: Close writer pipe in multiprocessing.Queue, n…