classification
Title: Exit threads when interpreter is finalizing rather than runtime.
Type: behavior Stage: patch review
Components: Interpreter Core Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, ncoghlan, pablogsal, vstinner
Priority: normal Keywords: patch

Created on 2019-03-29 21:59 by eric.snow, last changed 2019-05-10 02:06 by vstinner.

Pull Requests
URL Status Linked Edit
PR 12679 closed nanjekyejoannah, 2019-04-03 19:58
Messages (4)
msg339155 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-03-29 21:59
Currently when a thread acquires the GIL, it subsequently exits if the runtime is finalizing.  This helps with some cases like with stopping daemon threads.

This behavior should instead be triggered by the thread's interpreter finalizing rather than the runtime.  This implies the following changes:

* in Py_EndInterpreter() move "interp-finalizing = 1;" to right after the call to "wait_for_thread_shutdown()"
* in Py_FinalizeEx() add "interp-finalizing = 1;" right after the call to "wait_for_thread_shutdown()"
* update _PyEval_EvalFrameDefault() (the eval loop) to check "interp->finalizing" instead of the runtime
* likewise update PyEval_RestoreThread() (as well as PyEval_AcquireThread() and PyEval_AcquireLock(); see #36475)

The check will actually need to change from this:

  if (_Py_IsFinalizing() && !_Py_CURRENTLY_FINALIZING(tstate)) {

to look like this:

  if (interp->finalizing && !_Py_CURRENTLY_FINALIZING(tstate)) {

If we could guarantee that only the main thread will ever call Py_FinalizeEx() then it would look more like this:

  if (interp->finalizing && tstate.id != _PyRuntime.main_thread {
msg341882 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-08 14:57
Pablo pointed out a problem with this change at the PyCon sprints: the thread cleanup code doesn't currently distinguish between "Python created daemon thread" and "thread created by the embedding application".

That's already somewhat problematic if an embedding application goes through multiple initialize/finalize cycles, but it's even more critical to make the distinction correctly if we move the daemon thread cleanup to interpreter shutdown.
msg341884 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2019-05-08 15:04
Another potential issue here is if a thread belonging to another interpreter attempts to access the interpreter being cleaned up.
msg342039 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-05-10 02:06
IMHO PR 12679 change should wait until Py_EndInterpreter() is reworked to better cleanup its state and its threads.
History
Date User Action Args
2019-05-10 02:06:08vstinnersetmessages: + msg342039
2019-05-08 15:04:22ncoghlansetmessages: + msg341884
2019-05-08 14:57:24ncoghlansetnosy: + pablogsal, ncoghlan
messages: + msg341882
2019-04-29 08:54:56vstinnersetnosy: + vstinner
2019-04-03 19:58:38nanjekyejoannahsetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request12606
2019-03-29 21:59:02eric.snowcreate