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.

classification
Title: PyErr_Occurred(): tstate must be non-NULL
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: vstinner
Priority: normal Keywords: patch

Created on 2019-11-07 09:29 by vstinner, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 17080 merged vstinner, 2019-11-07 10:32
Messages (4)
msg356180 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 09:29
bpo-3605 modified PyErr_Occurred() to return NULL if the current Python thread state ("tstate") is NULL:

commit 8e0bdfd1d473ddffaf3501768678f8a970019da8
Author: Jeffrey Yasskin <jyasskin@gmail.com>
Date:   Thu May 13 18:31:05 2010 +0000

    Make PyErr_Occurred return NULL if there is no current thread.  Previously it
    would Py_FatalError, which called PyErr_Occurred, resulting in a semi-infinite
    recursion.
    
    Fixes issue 3605.

This change made PyErr_Occurred() inefficient: PyErr_Occurred() was a simple attribute read (tstate->curexc_type), and now there is an additional if per call.

I recently added _PyErr_Occurred(tstate) which is similar to PyErr_Occurred() but requires to pass tstate explicitly. I expected this function to be as simple as:

static inline PyObject* _PyErr_Occurred(PyThreadState *tstate)
{
    return tstate->curexc_type;
}

but the current implementation is:

static inline PyObject* _PyErr_Occurred(PyThreadState *tstate)
{
    return tstate == NULL ? NULL : tstate->curexc_type;
}


--

PyErr_Occurred() is currently implemented as:

PyObject* _Py_HOT_FUNCTION
PyErr_Occurred(void)
{
    PyThreadState *tstate = _PyThreadState_GET();
    return _PyErr_Occurred(tstate);
}

_PyThreadState_GET() must only be called with the GIL hold. This macro is designed to be efficient: it doesn't check if tstate is NULL.

_PyThreadState_GET() is only NULL if the thread has no Python thread state yet, or if the GIL is released.

IMHO PyErr_Occurred() must not be called if the GIL is released.
msg356184 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 11:42
New changeset d12d0e7c0fe2b49c40ac4d66365147c619d6c475 by Victor Stinner in branch 'master':
bpo-38733: PyErr_Occurred() caller must hold the GIL (GH-17080)
https://github.com/python/cpython/commit/d12d0e7c0fe2b49c40ac4d66365147c619d6c475
msg356186 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 11:45
In 2018 with bpo-26558, I fixed Py_FatalError() to no longer access the current exception of the current Python thread state, if the current thread doesn't not hold the GIL:

commit 3a228ab17c2a9cffd1a2f15f30d6209768de20a6
Author: Victor Stinner <vstinner@redhat.com>
Date:   Thu Nov 1 00:26:41 2018 +0100

    bpo-26558: Fix Py_FatalError() with GIL released (GH-10267)
    
    Don't call _Py_FatalError_PrintExc() nor flush_std_files() if the
    current thread doesn't hold the GIL, or if the current thread
    has no Python state thread.


Extract of Py_FatalError() code:

    /* Check if the current thread has a Python thread state
       and holds the GIL.

       tss_tstate is NULL if Py_FatalError() is called from a C thread which
       has no Python thread state.

       tss_tstate != tstate if the current Python thread does not hold the GIL.
       */
    PyThreadState *tss_tstate = PyGILState_GetThisThreadState();
    int has_tstate_and_gil = (tss_tstate != NULL && tss_tstate == tstate);
    if (has_tstate_and_gil) {
        /* If an exception is set, print the exception with its traceback */
        if (!_Py_FatalError_PrintExc(fd)) {
            /* No exception is set, or an exception is set without traceback */
            _Py_FatalError_DumpTracebacks(fd, interp, tss_tstate);
        }
    }
    else {
        _Py_FatalError_DumpTracebacks(fd, interp, tss_tstate);
    }
msg356187 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-11-07 11:47
I merged my change, I close the issue.

The change is a little bit backward incompatible, so I prefer to not backport it. PyErr_Occurred() checks tstate==NULL for 9 years, there is no urgency to change it :-) It can wait for Python 3.9.
History
Date User Action Args
2022-04-11 14:59:22adminsetgithub: 82914
2019-11-07 11:47:14vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg356187

stage: patch review -> resolved
2019-11-07 11:45:51vstinnersetmessages: + msg356186
2019-11-07 11:42:21vstinnersetmessages: + msg356184
2019-11-07 10:32:10vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request16589
2019-11-07 09:29:32vstinnercreate