From 6e443f48e71205e20555058596cdd9252e872ad1 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Mon, 30 Dec 2024 12:08:29 -0500 Subject: [PATCH 1/9] Use a portable assertion for holding the GIL --- Include/internal/pycore_pystate.h | 12 ++++++++++++ Modules/socketmodule.c | 8 ++++---- Objects/object.c | 4 ++-- Objects/obmalloc.c | 3 ++- Python/ceval_gil.c | 4 ++-- Python/errors.c | 4 ++-- Python/fileutils.c | 15 ++++++++------- Python/legacy_tracing.c | 8 ++++---- Python/pystate.c | 1 - Python/pytime.c | 13 +++++++------ Python/tracemalloc.c | 8 ++++---- 11 files changed, 47 insertions(+), 33 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 1e73e541ef8de0..700c24a8ab9f86 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -300,6 +300,18 @@ PyAPI_FUNC(const PyConfig*) _Py_GetConfig(void); // See also PyInterpreterState_Get() and _PyInterpreterState_GET(). extern PyInterpreterState* _PyGILState_GetInterpreterStateUnsafe(void); +#ifndef NDEBUG +/* Modern equivalent of assert(PyGILState_Check()) */ +static inline void +_Py_AssertHoldsTstate(void) +{ + PyThreadState *tstate = _PyThreadState_GET(); + _Py_EnsureTstateNotNULL(tstate); +} +#else +#define _Py_AssertHoldsTstate() +#endif + #ifdef __cplusplus } #endif diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 9394f1c940bedf..6c4d38e2215bcb 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -785,8 +785,8 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval, struct timeval tv, *tvp; #endif - /* must be called with the GIL held */ - assert(PyGILState_Check()); + /* must be called with a thread state */ + _Py_AssertHoldsTstate(); /* Error condition is for output only */ assert(!(connect && !writing)); @@ -899,8 +899,8 @@ sock_call_ex(PySocketSockObject *s, int deadline_initialized = 0; int res; - /* sock_call() must be called with the GIL held. */ - assert(PyGILState_Check()); + /* sock_call() must be called with a thread state. */ + _Py_AssertHoldsTstate(); /* outer loop to retry select() when select() is interrupted by a signal or to retry select()+sock_func() on false positive (see above) */ diff --git a/Objects/object.c b/Objects/object.c index d584414c559b9d..6372136a30ffba 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -3070,14 +3070,14 @@ _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt) } int PyRefTracer_SetTracer(PyRefTracer tracer, void *data) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); _PyRuntime.ref_tracer.tracer_func = tracer; _PyRuntime.ref_tracer.tracer_data = data; return 0; } PyRefTracer PyRefTracer_GetTracer(void** data) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); if (data != NULL) { *data = _PyRuntime.ref_tracer.tracer_data; } diff --git a/Objects/obmalloc.c b/Objects/obmalloc.c index b103deb01ca712..5688049b024696 100644 --- a/Objects/obmalloc.c +++ b/Objects/obmalloc.c @@ -2909,7 +2909,8 @@ _PyMem_DebugRawRealloc(void *ctx, void *p, size_t nbytes) static inline void _PyMem_DebugCheckGIL(const char *func) { - if (!PyGILState_Check()) { + PyThreadState *tstate = _PyThreadState_GET(); + if (tstate == NULL) { #ifndef Py_GIL_DISABLED _Py_FatalErrorFunc(func, "Python memory allocator called " diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 1f811e72406130..416eec01052224 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -995,7 +995,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) void _Py_FinishPendingCalls(PyThreadState *tstate) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); assert(_PyThreadState_CheckConsistency(tstate)); struct _pending_calls *pending = &tstate->interp->ceval.pending; @@ -1056,7 +1056,7 @@ _PyEval_MakePendingCalls(PyThreadState *tstate) int Py_MakePendingCalls(void) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); PyThreadState *tstate = _PyThreadState_GET(); assert(_PyThreadState_CheckConsistency(tstate)); diff --git a/Python/errors.c b/Python/errors.c index 2d362c1864ffff..e228655aa95174 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -314,8 +314,8 @@ _PyErr_SetLocaleString(PyObject *exception, const char *string) PyObject* _Py_HOT_FUNCTION PyErr_Occurred(void) { - /* The caller must hold the GIL. */ - assert(PyGILState_Check()); + /* The caller must hold a thread state. */ + _Py_AssertHoldsTstate(); PyThreadState *tstate = _PyThreadState_GET(); return _PyErr_Occurred(tstate); diff --git a/Python/fileutils.c b/Python/fileutils.c index 9529b14d377c60..3d7b35b0607f5a 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1,6 +1,7 @@ #include "Python.h" #include "pycore_fileutils.h" // fileutils definitions #include "pycore_runtime.h" // _PyRuntime +#include "pycore_pystate.h" // _Py_AssertHoldsTstate #include "osdefs.h" // SEP #include // mbstowcs() @@ -1311,7 +1312,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *status) { int res; - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); Py_BEGIN_ALLOW_THREADS res = _Py_fstat_noraise(fd, status); @@ -1691,7 +1692,7 @@ int _Py_open(const char *pathname, int flags) { /* _Py_open() must be called with the GIL held. */ - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); return _Py_open_impl(pathname, flags, 1); } @@ -1770,7 +1771,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) wchar_t wmode[10]; int usize; - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); if (PySys_Audit("open", "Osi", path, mode, 0) < 0) { return NULL; @@ -1806,7 +1807,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) PyObject *bytes; const char *path_bytes; - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); if (!PyUnicode_FSConverter(path, &bytes)) return NULL; @@ -1862,7 +1863,7 @@ _Py_read(int fd, void *buf, size_t count) int err; int async_err = 0; - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); /* _Py_read() must not be called with an exception set, otherwise the * caller may think that read() was interrupted by a signal and the signal @@ -2028,7 +2029,7 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) Py_ssize_t _Py_write(int fd, const void *buf, size_t count) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); /* _Py_write() must not be called with an exception set, otherwise the * caller may think that write() was interrupted by a signal and the signal @@ -2656,7 +2657,7 @@ _Py_dup(int fd) HANDLE handle; #endif - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); #ifdef MS_WINDOWS handle = _Py_get_osfhandle(fd); diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index 45af275f1f6dce..97634f9183c7d5 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -491,8 +491,8 @@ int _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); - /* The caller must hold the GIL */ - assert(PyGILState_Check()); + /* The caller must hold a thread state */ + _Py_AssertHoldsTstate(); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ @@ -586,8 +586,8 @@ int _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); - /* The caller must hold the GIL */ - assert(PyGILState_Check()); + /* The caller must hold a thread state */ + _Py_AssertHoldsTstate(); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ diff --git a/Python/pystate.c b/Python/pystate.c index c546b7c3a9f10e..52703b048d6022 100644 --- a/Python/pystate.c +++ b/Python/pystate.c @@ -2897,7 +2897,6 @@ _PyInterpreterState_GetConfigCopy(PyConfig *config) const PyConfig* _Py_GetConfig(void) { - assert(PyGILState_Check()); PyThreadState *tstate = current_fast_get(); _Py_EnsureTstateNotNULL(tstate); return _PyInterpreterState_GetConfig(tstate->interp); diff --git a/Python/pytime.c b/Python/pytime.c index 2b37cd991ef4e4..d9fd0c4329c39f 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,5 +1,6 @@ #include "Python.h" #include "pycore_time.h" // PyTime_t +#include "pycore_pystate.h" // _Py_AssertHoldsTstate #include // gmtime_r() #ifdef HAVE_SYS_TIME_H @@ -897,14 +898,14 @@ _PyTime_AsTimespec(PyTime_t t, struct timespec *ts) #endif -// N.B. If raise_exc=0, this may be called without the GIL. +// N.B. If raise_exc=0, this may be called without a thread state. static int py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) { assert(info == NULL || raise_exc); if (raise_exc) { - // raise_exc requires to hold the GIL - assert(PyGILState_Check()); + // raise_exc requires to hold a thread state + _Py_AssertHoldsTstate(); } #ifdef MS_WINDOWS @@ -1142,14 +1143,14 @@ py_mach_timebase_info(_PyTimeFraction *base, int raise_exc) #endif -// N.B. If raise_exc=0, this may be called without the GIL. +// N.B. If raise_exc=0, this may be called without a thread state. static int py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) { assert(info == NULL || raise_exc); if (raise_exc) { - // raise_exc requires to hold the GIL - assert(PyGILState_Check()); + // raise_exc requires to hold a thread state + _Py_AssertHoldsTstate(); } #if defined(MS_WINDOWS) diff --git a/Python/tracemalloc.c b/Python/tracemalloc.c index f661d69c0312fa..e8b9dc1f13c6e5 100644 --- a/Python/tracemalloc.c +++ b/Python/tracemalloc.c @@ -364,7 +364,7 @@ traceback_new(void) traceback_t *traceback; _Py_hashtable_entry_t *entry; - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); /* get frames */ traceback = tracemalloc_traceback; @@ -782,7 +782,7 @@ static void tracemalloc_clear_traces(void) { /* The GIL protects variables against concurrent access */ - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); TABLES_LOCK(); _Py_hashtable_clear(tracemalloc_traces); @@ -1345,7 +1345,7 @@ PyTraceMalloc_Untrack(unsigned int domain, uintptr_t ptr) void _PyTraceMalloc_Fini(void) { - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); tracemalloc_deinit(); } @@ -1362,7 +1362,7 @@ _PyTraceMalloc_TraceRef(PyObject *op, PyRefTracerEvent event, void* Py_UNUSED(ig return 0; } - assert(PyGILState_Check()); + _Py_AssertHoldsTstate(); if (!tracemalloc_config.tracing) { /* tracemalloc is not tracing: do nothing */ From a5aca2155551e468521bceb068a117f1ef0d8d11 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 7 Jan 2025 12:49:52 -0500 Subject: [PATCH 2/9] Switch to a utility function instead of an assertion --- Include/internal/pycore_pystate.h | 13 ++++--------- Modules/socketmodule.c | 4 ++-- Objects/object.c | 4 ++-- Python/ceval_gil.c | 4 ++-- Python/errors.c | 2 +- Python/fileutils.c | 16 ++++++++-------- Python/legacy_tracing.c | 4 ++-- Python/pytime.c | 6 +++--- Python/tracemalloc.c | 8 ++++---- 9 files changed, 28 insertions(+), 33 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index 700c24a8ab9f86..e5ce07ed44530d 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -300,17 +300,12 @@ PyAPI_FUNC(const PyConfig*) _Py_GetConfig(void); // See also PyInterpreterState_Get() and _PyInterpreterState_GET(). extern PyInterpreterState* _PyGILState_GetInterpreterStateUnsafe(void); -#ifndef NDEBUG -/* Modern equivalent of assert(PyGILState_Check()) */ -static inline void -_Py_AssertHoldsTstate(void) +/* Modern equivalent of PyGILState_Check() */ +static inline int +_Py_HoldsTstate(void) { - PyThreadState *tstate = _PyThreadState_GET(); - _Py_EnsureTstateNotNULL(tstate); + return _PyThreadState_GET() != _Py_NULL; } -#else -#define _Py_AssertHoldsTstate() -#endif #ifdef __cplusplus } diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 6c4d38e2215bcb..d92310d3d5949e 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -786,7 +786,7 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval, #endif /* must be called with a thread state */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* Error condition is for output only */ assert(!(connect && !writing)); @@ -900,7 +900,7 @@ sock_call_ex(PySocketSockObject *s, int res; /* sock_call() must be called with a thread state. */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* outer loop to retry select() when select() is interrupted by a signal or to retry select()+sock_func() on false positive (see above) */ diff --git a/Objects/object.c b/Objects/object.c index 6372136a30ffba..5746c5cf446bd9 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -3070,14 +3070,14 @@ _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt) } int PyRefTracer_SetTracer(PyRefTracer tracer, void *data) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); _PyRuntime.ref_tracer.tracer_func = tracer; _PyRuntime.ref_tracer.tracer_data = data; return 0; } PyRefTracer PyRefTracer_GetTracer(void** data) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); if (data != NULL) { *data = _PyRuntime.ref_tracer.tracer_data; } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 416eec01052224..7814b819aa5edd 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -995,7 +995,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) void _Py_FinishPendingCalls(PyThreadState *tstate) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); assert(_PyThreadState_CheckConsistency(tstate)); struct _pending_calls *pending = &tstate->interp->ceval.pending; @@ -1056,7 +1056,7 @@ _PyEval_MakePendingCalls(PyThreadState *tstate) int Py_MakePendingCalls(void) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); PyThreadState *tstate = _PyThreadState_GET(); assert(_PyThreadState_CheckConsistency(tstate)); diff --git a/Python/errors.c b/Python/errors.c index e228655aa95174..8f38523bdff233 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -315,7 +315,7 @@ PyObject* _Py_HOT_FUNCTION PyErr_Occurred(void) { /* The caller must hold a thread state. */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); PyThreadState *tstate = _PyThreadState_GET(); return _PyErr_Occurred(tstate); diff --git a/Python/fileutils.c b/Python/fileutils.c index 3d7b35b0607f5a..33fbda559b0855 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1,7 +1,7 @@ #include "Python.h" #include "pycore_fileutils.h" // fileutils definitions #include "pycore_runtime.h" // _PyRuntime -#include "pycore_pystate.h" // _Py_AssertHoldsTstate +#include "pycore_pystate.h" // _Py_HoldsTstate #include "osdefs.h" // SEP #include // mbstowcs() @@ -1312,7 +1312,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *status) { int res; - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); Py_BEGIN_ALLOW_THREADS res = _Py_fstat_noraise(fd, status); @@ -1692,7 +1692,7 @@ int _Py_open(const char *pathname, int flags) { /* _Py_open() must be called with the GIL held. */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); return _Py_open_impl(pathname, flags, 1); } @@ -1771,7 +1771,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) wchar_t wmode[10]; int usize; - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); if (PySys_Audit("open", "Osi", path, mode, 0) < 0) { return NULL; @@ -1807,7 +1807,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) PyObject *bytes; const char *path_bytes; - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); if (!PyUnicode_FSConverter(path, &bytes)) return NULL; @@ -1863,7 +1863,7 @@ _Py_read(int fd, void *buf, size_t count) int err; int async_err = 0; - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* _Py_read() must not be called with an exception set, otherwise the * caller may think that read() was interrupted by a signal and the signal @@ -2029,7 +2029,7 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) Py_ssize_t _Py_write(int fd, const void *buf, size_t count) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* _Py_write() must not be called with an exception set, otherwise the * caller may think that write() was interrupted by a signal and the signal @@ -2657,7 +2657,7 @@ _Py_dup(int fd) HANDLE handle; #endif - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); #ifdef MS_WINDOWS handle = _Py_get_osfhandle(fd); diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index 97634f9183c7d5..6683a71c38631a 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -492,7 +492,7 @@ _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); /* The caller must hold a thread state */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ @@ -587,7 +587,7 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); /* The caller must hold a thread state */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ diff --git a/Python/pytime.c b/Python/pytime.c index d9fd0c4329c39f..587b444b6d3ba9 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,6 +1,6 @@ #include "Python.h" #include "pycore_time.h" // PyTime_t -#include "pycore_pystate.h" // _Py_AssertHoldsTstate +#include "pycore_pystate.h" // _Py_HoldsTstate #include // gmtime_r() #ifdef HAVE_SYS_TIME_H @@ -905,7 +905,7 @@ py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) assert(info == NULL || raise_exc); if (raise_exc) { // raise_exc requires to hold a thread state - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); } #ifdef MS_WINDOWS @@ -1150,7 +1150,7 @@ py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) assert(info == NULL || raise_exc); if (raise_exc) { // raise_exc requires to hold a thread state - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); } #if defined(MS_WINDOWS) diff --git a/Python/tracemalloc.c b/Python/tracemalloc.c index e8b9dc1f13c6e5..46d76faf67ece7 100644 --- a/Python/tracemalloc.c +++ b/Python/tracemalloc.c @@ -364,7 +364,7 @@ traceback_new(void) traceback_t *traceback; _Py_hashtable_entry_t *entry; - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); /* get frames */ traceback = tracemalloc_traceback; @@ -782,7 +782,7 @@ static void tracemalloc_clear_traces(void) { /* The GIL protects variables against concurrent access */ - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); TABLES_LOCK(); _Py_hashtable_clear(tracemalloc_traces); @@ -1345,7 +1345,7 @@ PyTraceMalloc_Untrack(unsigned int domain, uintptr_t ptr) void _PyTraceMalloc_Fini(void) { - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); tracemalloc_deinit(); } @@ -1362,7 +1362,7 @@ _PyTraceMalloc_TraceRef(PyObject *op, PyRefTracerEvent event, void* Py_UNUSED(ig return 0; } - _Py_AssertHoldsTstate(); + assert(_Py_HoldsTstate()); if (!tracemalloc_config.tracing) { /* tracemalloc is not tracing: do nothing */ From c29736404191f7ce2065b09ee0ac3e6f72fec5d0 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 7 Jan 2025 14:15:19 -0500 Subject: [PATCH 3/9] Remove whitespace. --- Python/fileutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/fileutils.c b/Python/fileutils.c index ced56cf3d9b90b..fa16776f345b7d 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1772,7 +1772,7 @@ Py_fopen(PyObject *path, const char *mode) if (PySys_Audit("open", "Osi", path, mode, 0) < 0) { return NULL; } - + FILE *f; int async_err = 0; int saved_errno; From 7cb2d1e105f118d3cc3abb709ec94d5d7fe1a572 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Tue, 7 Jan 2025 15:51:22 -0500 Subject: [PATCH 4/9] Fix failing build. --- Modules/socketmodule.c | 1 + 1 file changed, 1 insertion(+) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index ec3c12c2b4c188..55f67eaa524248 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -110,6 +110,7 @@ Local naming conventions: #include "pycore_fileutils.h" // _Py_set_inheritable() #include "pycore_moduleobject.h" // _PyModule_GetState #include "pycore_time.h" // _PyTime_AsMilliseconds() +#include "pycore_pystate.h" // _Py_HoldsTstate() #include "pycore_pyatomic_ft_wrappers.h" #ifdef _Py_MEMORY_SANITIZER From e37e78bb79045f9da18cf97d65bc8df1d0eba237 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Wed, 8 Jan 2025 11:28:53 -0500 Subject: [PATCH 5/9] Always display error message. --- Include/internal/pycore_pystate.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index e5ce07ed44530d..f926d64c38e47f 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -300,11 +300,13 @@ PyAPI_FUNC(const PyConfig*) _Py_GetConfig(void); // See also PyInterpreterState_Get() and _PyInterpreterState_GET(). extern PyInterpreterState* _PyGILState_GetInterpreterStateUnsafe(void); -/* Modern equivalent of PyGILState_Check() */ +/* Modern equivalent of PyGILState_Check() for use in assertions only. */ static inline int _Py_HoldsTstate(void) { - return _PyThreadState_GET() != _Py_NULL; + PyThreadState *tstate = _PyThreadState_GET(); + _Py_EnsureTstateNotNULL(tstate); + return 1; } #ifdef __cplusplus From 6e186f387128a146853c577775aaae0477b30c80 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 19 Jan 2025 12:00:21 -0500 Subject: [PATCH 6/9] Switch back to _Py_AssertHoldsTstate() --- Include/internal/pycore_pystate.h | 14 +++++++++----- Modules/socketmodule.c | 4 ++-- Objects/object.c | 4 ++-- Python/ceval_gil.c | 4 ++-- Python/errors.c | 2 +- Python/fileutils.c | 12 ++++++------ Python/legacy_tracing.c | 4 ++-- Python/pytime.c | 4 ++-- Python/tracemalloc.c | 8 ++++---- 9 files changed, 30 insertions(+), 26 deletions(-) diff --git a/Include/internal/pycore_pystate.h b/Include/internal/pycore_pystate.h index f926d64c38e47f..ff3b222b157810 100644 --- a/Include/internal/pycore_pystate.h +++ b/Include/internal/pycore_pystate.h @@ -300,14 +300,18 @@ PyAPI_FUNC(const PyConfig*) _Py_GetConfig(void); // See also PyInterpreterState_Get() and _PyInterpreterState_GET(). extern PyInterpreterState* _PyGILState_GetInterpreterStateUnsafe(void); -/* Modern equivalent of PyGILState_Check() for use in assertions only. */ -static inline int -_Py_HoldsTstate(void) +#ifndef NDEBUG +/* Modern equivalent of assert(PyGILState_Check()) */ +static inline void +_Py_AssertHoldsTstateFunc(const char *func) { PyThreadState *tstate = _PyThreadState_GET(); - _Py_EnsureTstateNotNULL(tstate); - return 1; + _Py_EnsureFuncTstateNotNULL(func, tstate); } +#define _Py_AssertHoldsTstate() _Py_AssertHoldsTstateFunc(__func__) +#else +#define _Py_AssertHoldsTstate() +#endif #ifdef __cplusplus } diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 55f67eaa524248..150e4d9d51dbfc 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -824,7 +824,7 @@ internal_select(PySocketSockObject *s, int writing, PyTime_t interval, #endif /* must be called with a thread state */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* Error condition is for output only */ assert(!(connect && !writing)); @@ -938,7 +938,7 @@ sock_call_ex(PySocketSockObject *s, int res; /* sock_call() must be called with a thread state. */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* outer loop to retry select() when select() is interrupted by a signal or to retry select()+sock_func() on false positive (see above) */ diff --git a/Objects/object.c b/Objects/object.c index 71e92854f78f0e..dc39286a8f5daf 100644 --- a/Objects/object.c +++ b/Objects/object.c @@ -3074,14 +3074,14 @@ _Py_SetRefcnt(PyObject *ob, Py_ssize_t refcnt) } int PyRefTracer_SetTracer(PyRefTracer tracer, void *data) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); _PyRuntime.ref_tracer.tracer_func = tracer; _PyRuntime.ref_tracer.tracer_data = data; return 0; } PyRefTracer PyRefTracer_GetTracer(void** data) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); if (data != NULL) { *data = _PyRuntime.ref_tracer.tracer_data; } diff --git a/Python/ceval_gil.c b/Python/ceval_gil.c index 7814b819aa5edd..416eec01052224 100644 --- a/Python/ceval_gil.c +++ b/Python/ceval_gil.c @@ -995,7 +995,7 @@ _Py_unset_eval_breaker_bit_all(PyInterpreterState *interp, uintptr_t bit) void _Py_FinishPendingCalls(PyThreadState *tstate) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); assert(_PyThreadState_CheckConsistency(tstate)); struct _pending_calls *pending = &tstate->interp->ceval.pending; @@ -1056,7 +1056,7 @@ _PyEval_MakePendingCalls(PyThreadState *tstate) int Py_MakePendingCalls(void) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); PyThreadState *tstate = _PyThreadState_GET(); assert(_PyThreadState_CheckConsistency(tstate)); diff --git a/Python/errors.c b/Python/errors.c index 14ad5f145bbacd..9c7b771133dcf4 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -315,7 +315,7 @@ PyObject* _Py_HOT_FUNCTION PyErr_Occurred(void) { /* The caller must hold a thread state. */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); PyThreadState *tstate = _PyThreadState_GET(); return _PyErr_Occurred(tstate); diff --git a/Python/fileutils.c b/Python/fileutils.c index fa16776f345b7d..6c599e70e6edaf 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1312,7 +1312,7 @@ _Py_fstat(int fd, struct _Py_stat_struct *status) { int res; - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); Py_BEGIN_ALLOW_THREADS res = _Py_fstat_noraise(fd, status); @@ -1692,7 +1692,7 @@ int _Py_open(const char *pathname, int flags) { /* _Py_open() must be called with the GIL held. */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); return _Py_open_impl(pathname, flags, 1); } @@ -1767,7 +1767,7 @@ _Py_wfopen(const wchar_t *path, const wchar_t *mode) FILE* Py_fopen(PyObject *path, const char *mode) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); if (PySys_Audit("open", "Osi", path, mode, 0) < 0) { return NULL; @@ -1882,7 +1882,7 @@ _Py_read(int fd, void *buf, size_t count) int err; int async_err = 0; - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* _Py_read() must not be called with an exception set, otherwise the * caller may think that read() was interrupted by a signal and the signal @@ -2048,7 +2048,7 @@ _Py_write_impl(int fd, const void *buf, size_t count, int gil_held) Py_ssize_t _Py_write(int fd, const void *buf, size_t count) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* _Py_write() must not be called with an exception set, otherwise the * caller may think that write() was interrupted by a signal and the signal @@ -2676,7 +2676,7 @@ _Py_dup(int fd) HANDLE handle; #endif - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); #ifdef MS_WINDOWS handle = _Py_get_osfhandle(fd); diff --git a/Python/legacy_tracing.c b/Python/legacy_tracing.c index 6683a71c38631a..97634f9183c7d5 100644 --- a/Python/legacy_tracing.c +++ b/Python/legacy_tracing.c @@ -492,7 +492,7 @@ _PyEval_SetProfile(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); /* The caller must hold a thread state */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ @@ -587,7 +587,7 @@ _PyEval_SetTrace(PyThreadState *tstate, Py_tracefunc func, PyObject *arg) { assert(is_tstate_valid(tstate)); /* The caller must hold a thread state */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* Call _PySys_Audit() in the context of the current thread state, even if tstate is not the current thread state. */ diff --git a/Python/pytime.c b/Python/pytime.c index 587b444b6d3ba9..219b0bc8fe2968 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -905,7 +905,7 @@ py_get_system_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) assert(info == NULL || raise_exc); if (raise_exc) { // raise_exc requires to hold a thread state - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); } #ifdef MS_WINDOWS @@ -1150,7 +1150,7 @@ py_get_monotonic_clock(PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) assert(info == NULL || raise_exc); if (raise_exc) { // raise_exc requires to hold a thread state - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); } #if defined(MS_WINDOWS) diff --git a/Python/tracemalloc.c b/Python/tracemalloc.c index 46d76faf67ece7..e8b9dc1f13c6e5 100644 --- a/Python/tracemalloc.c +++ b/Python/tracemalloc.c @@ -364,7 +364,7 @@ traceback_new(void) traceback_t *traceback; _Py_hashtable_entry_t *entry; - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); /* get frames */ traceback = tracemalloc_traceback; @@ -782,7 +782,7 @@ static void tracemalloc_clear_traces(void) { /* The GIL protects variables against concurrent access */ - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); TABLES_LOCK(); _Py_hashtable_clear(tracemalloc_traces); @@ -1345,7 +1345,7 @@ PyTraceMalloc_Untrack(unsigned int domain, uintptr_t ptr) void _PyTraceMalloc_Fini(void) { - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); tracemalloc_deinit(); } @@ -1362,7 +1362,7 @@ _PyTraceMalloc_TraceRef(PyObject *op, PyRefTracerEvent event, void* Py_UNUSED(ig return 0; } - assert(_Py_HoldsTstate()); + _Py_AssertHoldsTstate(); if (!tracemalloc_config.tracing) { /* tracemalloc is not tracing: do nothing */ From 5f82d1cb8789ae9dd7fbd719834df06175ba003d Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 19 Jan 2025 12:24:59 -0500 Subject: [PATCH 7/9] Update Python/fileutils.c Co-authored-by: Kumar Aditya --- Python/fileutils.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/fileutils.c b/Python/fileutils.c index 6c599e70e6edaf..72804c39220591 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -1,7 +1,7 @@ #include "Python.h" #include "pycore_fileutils.h" // fileutils definitions #include "pycore_runtime.h" // _PyRuntime -#include "pycore_pystate.h" // _Py_HoldsTstate +#include "pycore_pystate.h" // _Py_AssertHoldsTstate() #include "osdefs.h" // SEP #include // mbstowcs() From 63a09bf14aa223c476d2ef8dcc6eade0372ebff5 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 19 Jan 2025 12:25:05 -0500 Subject: [PATCH 8/9] Update Modules/socketmodule.c Co-authored-by: Kumar Aditya --- Modules/socketmodule.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/socketmodule.c b/Modules/socketmodule.c index 150e4d9d51dbfc..5e81253ca4a591 100644 --- a/Modules/socketmodule.c +++ b/Modules/socketmodule.c @@ -110,7 +110,7 @@ Local naming conventions: #include "pycore_fileutils.h" // _Py_set_inheritable() #include "pycore_moduleobject.h" // _PyModule_GetState #include "pycore_time.h" // _PyTime_AsMilliseconds() -#include "pycore_pystate.h" // _Py_HoldsTstate() +#include "pycore_pystate.h" // _Py_AssertHoldsTstate() #include "pycore_pyatomic_ft_wrappers.h" #ifdef _Py_MEMORY_SANITIZER From 0bcdc4cee5ebd0a3d9d90010625403607b18baf1 Mon Sep 17 00:00:00 2001 From: Peter Bierma Date: Sun, 19 Jan 2025 12:29:20 -0500 Subject: [PATCH 9/9] Update Python/pytime.c Co-authored-by: Kumar Aditya --- Python/pytime.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/pytime.c b/Python/pytime.c index 219b0bc8fe2968..c039fc98ce4bde 100644 --- a/Python/pytime.c +++ b/Python/pytime.c @@ -1,6 +1,6 @@ #include "Python.h" #include "pycore_time.h" // PyTime_t -#include "pycore_pystate.h" // _Py_HoldsTstate +#include "pycore_pystate.h" // _Py_AssertHoldsTstate() #include // gmtime_r() #ifdef HAVE_SYS_TIME_H