This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author vstinner
Recipients Arfrever, larry, neologix, pitrou, python-dev, sebastien.renard, serhiy.storchaka, vstinner
Date 2020-03-23.14:43:28
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1584974609.62.0.876680884643.issue20526@roundup.psfhosted.org>
In-reply-to
Content
Right now, threading_shutdown_interrupted.py does crash on the master branch (commit 05e4a296ecc127641160a04f39cc02c0f66a8c27). I reintroduced the bug with:

commit 9ad58acbe8b90b4d0f2d2e139e38bb5aa32b7fb6
Author: Victor Stinner <vstinner@python.org>
Date:   Mon Mar 9 23:37:49 2020 +0100

    bpo-19466: Py_Finalize() clears daemon threads earlier (GH-18848)
    
    Clear the frames of daemon threads earlier during the Python shutdown to
    call objects destructors. So "unclosed file" resource warnings are now
    emitted for daemon threads in a more reliable way.

I investigated this bug with Pablo Galindo Salgado. When the bug triggers, there are 2 strong references to a Frame object:

* Traceback object
* Generator object

The bug occurs in "except ValueError as exc:" of EvilThread.coroutine().

The exception contains a traceback object which has a strong reference to the frame.

The frame object contains "exc" variable (the exception) which contains a reference to the tracecback (exc.__traceback__) which contains a refererence to the frame (tb.tb_frame): there is a reference cycle. (It's not really an issue by itself).

There are 2 strong references to the Frame object.

Py_FinalizeEx() calls _PyThreadState_DeleteExcept() which doesn't clear the reference cycle. But PyThreadState_Clear() calls Py_CLEAR(tstate->frame) which reduces the Frame reference counter from 2 to 1, even if there are still 2 strong references to it (traceback and generator).

tstate->frame is only set at 3 places:

* new_threadstate(): when a Python thread state is created (it's initialized to NULL)
* _PyEval_EvalFrameDefault() entry: set to frame
* _PyEval_EvalFrameDefault() exit: set to frame->f_back (which can be NULL

By the way, _PyEval_EvalFrameDefault() has a bug: tstate->frame is not reset if _PyCode_InitOpcache() fails: bpo-40048.

Py_FinalizeEx() calls _PyRuntimeState_SetFinalizing(runtime, tstate) and so threads which tries to take the GIL must exit immediately. Extract of take_gil():
---
static inline int
tstate_must_exit(PyThreadState *tstate)
{
    PyThreadState *finalizing = _PyRuntimeState_GetFinalizing(&_PyRuntime);
    return (finalizing != NULL && finalizing != tstate);
}

static void
take_gil(PyThreadState *tstate)
{
    ...

    if (tstate_must_exit(tstate)) {
        PyThread_exit_thread();
    }
    ...
}
---

The thread is exited in the middle of _PyEval_EvalFrameDefault() by take_gil() which calls PyThread_exit_thread() because the thread must exit (because the main thread destroyed its Python thread state and so accessing it would crash the thread).

_PyThreadState_DeleteExcept() calls PyThreadState_Clear() which calls Py_CLEAR(tstate->frame): this case only occurs because the thread exited in the middle of _PyEval_EvalFrameDefault(). In the normal case, _PyEval_EvalFrameDefault() always resets tstate->frame to its previous value.

Py_CLEAR(tstate->frame) is wrong: it's a *borrowed* reference.

Pablo and me agree with PyThreadState_Clear() must not call Py_CLEAR(tstate->frame): it's borrowed reference, not a strong reference.

I'm working on a PR to fix the issue.
History
Date User Action Args
2020-03-23 14:43:29vstinnersetrecipients: + vstinner, pitrou, larry, Arfrever, neologix, python-dev, serhiy.storchaka, sebastien.renard
2020-03-23 14:43:29vstinnersetmessageid: <1584974609.62.0.876680884643.issue20526@roundup.psfhosted.org>
2020-03-23 14:43:29vstinnerlinkissue20526 messages
2020-03-23 14:43:28vstinnercreate