From 42e98b8ab48b09c63b60aeeada7181bbede1fb66 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 18 Mar 2024 15:38:00 +0100 Subject: [PATCH 1/7] gh-116664: Make module state Py_SETREF's in _warnings thread-safe Mark the swap operations as critical sections. --- Python/_warnings.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Python/_warnings.c b/Python/_warnings.c index dfa82c569e1383..ae584c7279cfeb 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1,4 +1,5 @@ #include "Python.h" +#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() #include "pycore_interp.h" // PyInterpreterState.warnings #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_pyerrors.h" // _PyErr_Occurred() @@ -257,7 +258,9 @@ get_once_registry(PyInterpreterState *interp) Py_DECREF(registry); return NULL; } + Py_BEGIN_CRITICAL_SECTION(registry); Py_SETREF(st->once_registry, registry); + Py_END_CRITICAL_SECTION(); return registry; } @@ -288,7 +291,9 @@ get_default_action(PyInterpreterState *interp) Py_DECREF(default_action); return NULL; } + Py_BEGIN_CRITICAL_SECTION(default_action); Py_SETREF(st->default_action, default_action); + Py_END_CRITICAL_SECTION(); return default_action; } @@ -313,7 +318,9 @@ get_filter(PyInterpreterState *interp, PyObject *category, return NULL; } else { + Py_BEGIN_CRITICAL_SECTION(warnings_filters); Py_SETREF(st->filters, warnings_filters); + Py_END_CRITICAL_SECTION(); } PyObject *filters = st->filters; From d222f7c83ed1dfe53b47eabce9e0ccaefdb6a83c Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Mon, 18 Mar 2024 23:46:58 +0100 Subject: [PATCH 2/7] Protect warn_explicit() instead --- Python/_warnings.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/Python/_warnings.c b/Python/_warnings.c index ae584c7279cfeb..ad2cf0c5a79589 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -236,14 +236,14 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) static PyObject * get_once_registry(PyInterpreterState *interp) { - PyObject *registry; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } - registry = GET_WARNINGS_ATTR(interp, onceregistry, 0); + PyObject *registry = GET_WARNINGS_ATTR(interp, onceregistry, 0); if (registry == NULL) { if (PyErr_Occurred()) return NULL; @@ -258,9 +258,7 @@ get_once_registry(PyInterpreterState *interp) Py_DECREF(registry); return NULL; } - Py_BEGIN_CRITICAL_SECTION(registry); Py_SETREF(st->once_registry, registry); - Py_END_CRITICAL_SECTION(); return registry; } @@ -268,14 +266,14 @@ get_once_registry(PyInterpreterState *interp) static PyObject * get_default_action(PyInterpreterState *interp) { - PyObject *default_action; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } - default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0); + PyObject *default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0); if (default_action == NULL) { if (PyErr_Occurred()) { return NULL; @@ -291,9 +289,7 @@ get_default_action(PyInterpreterState *interp) Py_DECREF(default_action); return NULL; } - Py_BEGIN_CRITICAL_SECTION(default_action); Py_SETREF(st->default_action, default_action); - Py_END_CRITICAL_SECTION(); return default_action; } @@ -304,23 +300,20 @@ get_filter(PyInterpreterState *interp, PyObject *category, PyObject *text, Py_ssize_t lineno, PyObject *module, PyObject **item) { - PyObject *action; - Py_ssize_t i; - PyObject *warnings_filters; + _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); + WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } - warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0); + PyObject *warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0); if (warnings_filters == NULL) { if (PyErr_Occurred()) return NULL; } else { - Py_BEGIN_CRITICAL_SECTION(warnings_filters); Py_SETREF(st->filters, warnings_filters); - Py_END_CRITICAL_SECTION(); } PyObject *filters = st->filters; @@ -331,7 +324,7 @@ get_filter(PyInterpreterState *interp, PyObject *category, } /* WarningsState.filters could change while we are iterating over it. */ - for (i = 0; i < PyList_GET_SIZE(filters); i++) { + for (Py_ssize_t i = 0; i < PyList_GET_SIZE(filters); i++) { PyObject *tmp_item, *action, *msg, *cat, *mod, *ln_obj; Py_ssize_t ln; int is_subclass, good_msg, good_mod; @@ -391,7 +384,7 @@ get_filter(PyInterpreterState *interp, PyObject *category, Py_DECREF(tmp_item); } - action = get_default_action(interp); + PyObject *action = get_default_action(interp); if (action != NULL) { *item = Py_NewRef(Py_None); return action; @@ -1007,8 +1000,10 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, &filename, &lineno, &module, ®istry)) return NULL; + Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, source); + Py_END_CRITICAL_SECTION(); Py_DECREF(filename); Py_DECREF(registry); Py_DECREF(module); @@ -1156,8 +1151,10 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, return NULL; } } + Py_BEGIN_CRITICAL_SECTION(tstate->interp); returned = warn_explicit(tstate, category, message, filename, lineno, mod, registry, source_line, sourceobj); + Py_END_CRITICAL_SECTION(); Py_XDECREF(source_line); return returned; } @@ -1363,8 +1360,10 @@ PyErr_WarnExplicitFormat(PyObject *category, PyObject *res; PyThreadState *tstate = get_current_tstate(); if (tstate != NULL) { + Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); + Py_END_CRITICAL_SECTION(); Py_DECREF(message); if (res != NULL) { Py_DECREF(res); From be08b222a866c0bc8eeb266023d853a4452720da Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Tue, 19 Mar 2024 10:10:48 +0100 Subject: [PATCH 3/7] Forgot one call --- Python/_warnings.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Python/_warnings.c b/Python/_warnings.c index ad2cf0c5a79589..2412fd1176d825 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1294,8 +1294,10 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, if (tstate == NULL) { return -1; } + Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); + Py_END_CRITICAL_SECTION(); if (res == NULL) return -1; Py_DECREF(res); From a87d84c5ec60a41b4821bb4d80c4b366e1916b63 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 20 Mar 2024 00:09:23 +0100 Subject: [PATCH 4/7] Use more granular critical sections --- Python/_warnings.c | 76 ++++++++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/Python/_warnings.c b/Python/_warnings.c index 2412fd1176d825..311baa3fc68a99 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -234,15 +234,8 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) static PyObject * -get_once_registry(PyInterpreterState *interp) +get_once_registry_impl(PyInterpreterState *interp, WarningsState *st) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); - - WarningsState *st = warnings_get_state(interp); - if (st == NULL) { - return NULL; - } - PyObject *registry = GET_WARNINGS_ATTR(interp, onceregistry, 0); if (registry == NULL) { if (PyErr_Occurred()) @@ -264,15 +257,25 @@ get_once_registry(PyInterpreterState *interp) static PyObject * -get_default_action(PyInterpreterState *interp) +get_once_registry(PyInterpreterState *interp) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); - WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } + PyObject *registry; + Py_BEGIN_CRITICAL_SECTION(st->once_registry); + registry = get_once_registry_impl(interp, st); + Py_END_CRITICAL_SECTION(); + + return registry; +} + + +static PyObject * +get_default_action_impl(PyInterpreterState *interp, WarningsState *st) +{ PyObject *default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0); if (default_action == NULL) { if (PyErr_Occurred()) { @@ -294,19 +297,29 @@ get_default_action(PyInterpreterState *interp) } -/* The item is a new reference. */ -static PyObject* -get_filter(PyInterpreterState *interp, PyObject *category, - PyObject *text, Py_ssize_t lineno, - PyObject *module, PyObject **item) +static PyObject * +get_default_action(PyInterpreterState *interp) { - _Py_CRITICAL_SECTION_ASSERT_OBJECT_LOCKED(interp); - WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } + PyObject *default_action; + Py_BEGIN_CRITICAL_SECTION(st->default_action); + default_action = get_default_action_impl(interp, st); + Py_END_CRITICAL_SECTION(); + + return default_action; +} + + +/* The item is a new reference. */ +static PyObject * +get_filter_impl(PyInterpreterState *interp, WarningsState *st, + PyObject *category, PyObject *text, Py_ssize_t lineno, + PyObject *module, PyObject **item) +{ PyObject *warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0); if (warnings_filters == NULL) { if (PyErr_Occurred()) @@ -394,6 +407,25 @@ get_filter(PyInterpreterState *interp, PyObject *category, } +static PyObject * +get_filter(PyInterpreterState *interp, PyObject *category, + PyObject *text, Py_ssize_t lineno, + PyObject *module, PyObject **item) +{ + WarningsState *st = warnings_get_state(interp); + if (st == NULL) { + return NULL; + } + + PyObject *filters; + Py_BEGIN_CRITICAL_SECTION(st->filters); + filters = get_filter_impl(interp, st, category, text, lineno, module, item); + Py_END_CRITICAL_SECTION(); + + return filters; +} + + static int already_warned(PyInterpreterState *interp, PyObject *registry, PyObject *key, int should_set) @@ -1000,10 +1032,8 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, &filename, &lineno, &module, ®istry)) return NULL; - Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, source); - Py_END_CRITICAL_SECTION(); Py_DECREF(filename); Py_DECREF(registry); Py_DECREF(module); @@ -1151,10 +1181,8 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, return NULL; } } - Py_BEGIN_CRITICAL_SECTION(tstate->interp); returned = warn_explicit(tstate, category, message, filename, lineno, mod, registry, source_line, sourceobj); - Py_END_CRITICAL_SECTION(); Py_XDECREF(source_line); return returned; } @@ -1294,10 +1322,8 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, if (tstate == NULL) { return -1; } - Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); - Py_END_CRITICAL_SECTION(); if (res == NULL) return -1; Py_DECREF(res); @@ -1362,10 +1388,8 @@ PyErr_WarnExplicitFormat(PyObject *category, PyObject *res; PyThreadState *tstate = get_current_tstate(); if (tstate != NULL) { - Py_BEGIN_CRITICAL_SECTION(tstate->interp); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); - Py_END_CRITICAL_SECTION(); Py_DECREF(message); if (res != NULL) { Py_DECREF(res); From f9e549dd59c741b7f336560aa430e0d6c773b2c8 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 27 Mar 2024 21:14:44 +0100 Subject: [PATCH 5/7] Put a mutex in _warnings_runtime_state --- Include/internal/pycore_critical_section.h | 8 +- Include/internal/pycore_warnings.h | 1 + Python/_warnings.c | 108 ++++++++++----------- 3 files changed, 60 insertions(+), 57 deletions(-) diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 9163b5cf0f2e8a..3fb38989446f3b 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -87,10 +87,13 @@ extern "C" { #define _Py_CRITICAL_SECTION_MASK 0x3 #ifdef Py_GIL_DISABLED -# define Py_BEGIN_CRITICAL_SECTION(op) \ +# define Py_BEGIN_CRITICAL_SECTION_MUT(mutex) \ { \ _PyCriticalSection _cs; \ - _PyCriticalSection_Begin(&_cs, &_PyObject_CAST(op)->ob_mutex) + _PyCriticalSection_Begin(&_cs, &(mutex)) + +# define Py_BEGIN_CRITICAL_SECTION(op) \ + Py_BEGIN_CRITICAL_SECTION_MUT(_PyObject_CAST(op)->ob_mutex) # define Py_END_CRITICAL_SECTION() \ _PyCriticalSection_End(&_cs); \ @@ -138,6 +141,7 @@ extern "C" { #else /* !Py_GIL_DISABLED */ // The critical section APIs are no-ops with the GIL. +# define Py_BEGIN_CRITICAL_SECTION_MUT(mut) # define Py_BEGIN_CRITICAL_SECTION(op) # define Py_END_CRITICAL_SECTION() # define Py_XBEGIN_CRITICAL_SECTION(op) diff --git a/Include/internal/pycore_warnings.h b/Include/internal/pycore_warnings.h index 9785d7cc467de2..114796df42b2b6 100644 --- a/Include/internal/pycore_warnings.h +++ b/Include/internal/pycore_warnings.h @@ -14,6 +14,7 @@ struct _warnings_runtime_state { PyObject *filters; /* List */ PyObject *once_registry; /* Dict */ PyObject *default_action; /* String */ + struct _PyMutex mutex; long filters_version; }; diff --git a/Python/_warnings.c b/Python/_warnings.c index 311baa3fc68a99..0b830dce2438bd 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1,5 +1,5 @@ #include "Python.h" -#include "pycore_critical_section.h"// Py_BEGIN_CRITICAL_SECTION() +#include "pycore_critical_section.h" // Py_BEGIN_CRITICAL_SECTION_MUT() #include "pycore_interp.h" // PyInterpreterState.warnings #include "pycore_long.h" // _PyLong_GetZero() #include "pycore_pyerrors.h" // _PyErr_Occurred() @@ -133,6 +133,8 @@ _PyWarnings_InitState(PyInterpreterState *interp) { WarningsState *st = &interp->warnings; + st->mutex = (struct _PyMutex){ 0 }; + if (st->filters == NULL) { st->filters = init_filters(interp); if (st->filters == NULL) { @@ -234,8 +236,15 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import) static PyObject * -get_once_registry_impl(PyInterpreterState *interp, WarningsState *st) +get_once_registry(PyInterpreterState *interp) { + WarningsState *st = warnings_get_state(interp); + if (st == NULL) { + return NULL; + } + + _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); + PyObject *registry = GET_WARNINGS_ATTR(interp, onceregistry, 0); if (registry == NULL) { if (PyErr_Occurred()) @@ -257,25 +266,15 @@ get_once_registry_impl(PyInterpreterState *interp, WarningsState *st) static PyObject * -get_once_registry(PyInterpreterState *interp) +get_default_action(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } - PyObject *registry; - Py_BEGIN_CRITICAL_SECTION(st->once_registry); - registry = get_once_registry_impl(interp, st); - Py_END_CRITICAL_SECTION(); - - return registry; -} - + _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); -static PyObject * -get_default_action_impl(PyInterpreterState *interp, WarningsState *st) -{ PyObject *default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0); if (default_action == NULL) { if (PyErr_Occurred()) { @@ -297,29 +296,19 @@ get_default_action_impl(PyInterpreterState *interp, WarningsState *st) } -static PyObject * -get_default_action(PyInterpreterState *interp) +/* The item is a new reference. */ +static PyObject* +get_filter(PyInterpreterState *interp, PyObject *category, + PyObject *text, Py_ssize_t lineno, + PyObject *module, PyObject **item) { WarningsState *st = warnings_get_state(interp); if (st == NULL) { return NULL; } - PyObject *default_action; - Py_BEGIN_CRITICAL_SECTION(st->default_action); - default_action = get_default_action_impl(interp, st); - Py_END_CRITICAL_SECTION(); - - return default_action; -} + _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); - -/* The item is a new reference. */ -static PyObject * -get_filter_impl(PyInterpreterState *interp, WarningsState *st, - PyObject *category, PyObject *text, Py_ssize_t lineno, - PyObject *module, PyObject **item) -{ PyObject *warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0); if (warnings_filters == NULL) { if (PyErr_Occurred()) @@ -407,25 +396,6 @@ get_filter_impl(PyInterpreterState *interp, WarningsState *st, } -static PyObject * -get_filter(PyInterpreterState *interp, PyObject *category, - PyObject *text, Py_ssize_t lineno, - PyObject *module, PyObject **item) -{ - WarningsState *st = warnings_get_state(interp); - if (st == NULL) { - return NULL; - } - - PyObject *filters; - Py_BEGIN_CRITICAL_SECTION(st->filters); - filters = get_filter_impl(interp, st, category, text, lineno, module, item); - Py_END_CRITICAL_SECTION(); - - return filters; -} - - static int already_warned(PyInterpreterState *interp, PyObject *registry, PyObject *key, int should_set) @@ -1032,8 +1002,15 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, &filename, &lineno, &module, ®istry)) return NULL; + WarningsState *st = warnings_get_state(tstate->interp); + if (st == NULL) { + return NULL; + } + + Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, source); + Py_END_CRITICAL_SECTION(); Py_DECREF(filename); Py_DECREF(registry); Py_DECREF(module); @@ -1181,8 +1158,16 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, return NULL; } } + + WarningsState *st = warnings_get_state(tstate->interp); + if (st == NULL) { + return NULL; + } + + Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); returned = warn_explicit(tstate, category, message, filename, lineno, mod, registry, source_line, sourceobj); + Py_END_CRITICAL_SECTION(); Py_XDECREF(source_line); return returned; } @@ -1322,8 +1307,16 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, if (tstate == NULL) { return -1; } + + WarningsState *st = warnings_get_state(tstate->interp); + if (st == NULL) { + return -1; + } + + Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); + Py_END_CRITICAL_SECTION(); if (res == NULL) return -1; Py_DECREF(res); @@ -1388,12 +1381,17 @@ PyErr_WarnExplicitFormat(PyObject *category, PyObject *res; PyThreadState *tstate = get_current_tstate(); if (tstate != NULL) { - res = warn_explicit(tstate, category, message, filename, lineno, - module, registry, NULL, NULL); - Py_DECREF(message); - if (res != NULL) { - Py_DECREF(res); - ret = 0; + WarningsState *st = warnings_get_state(tstate->interp); + if (st != NULL) { + Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); + res = warn_explicit(tstate, category, message, filename, lineno, + module, registry, NULL, NULL); + Py_END_CRITICAL_SECTION(); + Py_DECREF(message); + if (res != NULL) { + Py_DECREF(res); + ret = 0; + } } } } From a6b127f75d46472c6ebc7b463db81a082769a641 Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Wed, 27 Mar 2024 21:54:07 +0100 Subject: [PATCH 6/7] Pass a pointer --- Include/internal/pycore_critical_section.h | 4 ++-- Python/_warnings.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h index 3fb38989446f3b..23b85c2f9e9bb2 100644 --- a/Include/internal/pycore_critical_section.h +++ b/Include/internal/pycore_critical_section.h @@ -90,10 +90,10 @@ extern "C" { # define Py_BEGIN_CRITICAL_SECTION_MUT(mutex) \ { \ _PyCriticalSection _cs; \ - _PyCriticalSection_Begin(&_cs, &(mutex)) + _PyCriticalSection_Begin(&_cs, mutex) # define Py_BEGIN_CRITICAL_SECTION(op) \ - Py_BEGIN_CRITICAL_SECTION_MUT(_PyObject_CAST(op)->ob_mutex) + Py_BEGIN_CRITICAL_SECTION_MUT(&_PyObject_CAST(op)->ob_mutex) # define Py_END_CRITICAL_SECTION() \ _PyCriticalSection_End(&_cs); \ diff --git a/Python/_warnings.c b/Python/_warnings.c index 0b830dce2438bd..94042840cd9b3b 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -1007,7 +1007,7 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, return NULL; } - Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); + Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, source); Py_END_CRITICAL_SECTION(); @@ -1164,7 +1164,7 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, return NULL; } - Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); + Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); returned = warn_explicit(tstate, category, message, filename, lineno, mod, registry, source_line, sourceobj); Py_END_CRITICAL_SECTION(); @@ -1313,7 +1313,7 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, return -1; } - Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); + Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); Py_END_CRITICAL_SECTION(); @@ -1383,7 +1383,7 @@ PyErr_WarnExplicitFormat(PyObject *category, if (tstate != NULL) { WarningsState *st = warnings_get_state(tstate->interp); if (st != NULL) { - Py_BEGIN_CRITICAL_SECTION_MUT(st->mutex); + Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, NULL, NULL); Py_END_CRITICAL_SECTION(); From c6c18662b48a20141c891dca008fc4fba67b3afd Mon Sep 17 00:00:00 2001 From: "Erlend E. Aasland" Date: Thu, 28 Mar 2024 15:35:36 +0100 Subject: [PATCH 7/7] Address review: assert state and remove unneeded mutex initialization --- Python/_warnings.c | 46 ++++++++++++++++------------------------------ 1 file changed, 16 insertions(+), 30 deletions(-) diff --git a/Python/_warnings.c b/Python/_warnings.c index 94042840cd9b3b..66a460e2a2c509 100644 --- a/Python/_warnings.c +++ b/Python/_warnings.c @@ -133,8 +133,6 @@ _PyWarnings_InitState(PyInterpreterState *interp) { WarningsState *st = &interp->warnings; - st->mutex = (struct _PyMutex){ 0 }; - if (st->filters == NULL) { st->filters = init_filters(interp); if (st->filters == NULL) { @@ -239,9 +237,7 @@ static PyObject * get_once_registry(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); - if (st == NULL) { - return NULL; - } + assert(st != NULL); _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); @@ -269,9 +265,7 @@ static PyObject * get_default_action(PyInterpreterState *interp) { WarningsState *st = warnings_get_state(interp); - if (st == NULL) { - return NULL; - } + assert(st != NULL); _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); @@ -303,9 +297,7 @@ get_filter(PyInterpreterState *interp, PyObject *category, PyObject *module, PyObject **item) { WarningsState *st = warnings_get_state(interp); - if (st == NULL) { - return NULL; - } + assert(st != NULL); _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex); @@ -1003,9 +995,7 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level, return NULL; WarningsState *st = warnings_get_state(tstate->interp); - if (st == NULL) { - return NULL; - } + assert(st != NULL); Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, module, registry, @@ -1160,9 +1150,7 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message, } WarningsState *st = warnings_get_state(tstate->interp); - if (st == NULL) { - return NULL; - } + assert(st != NULL); Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); returned = warn_explicit(tstate, category, message, filename, lineno, @@ -1309,9 +1297,7 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message, } WarningsState *st = warnings_get_state(tstate->interp); - if (st == NULL) { - return -1; - } + assert(st != NULL); Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); res = warn_explicit(tstate, category, message, filename, lineno, @@ -1382,16 +1368,16 @@ PyErr_WarnExplicitFormat(PyObject *category, PyThreadState *tstate = get_current_tstate(); if (tstate != NULL) { WarningsState *st = warnings_get_state(tstate->interp); - if (st != NULL) { - Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); - res = warn_explicit(tstate, category, message, filename, lineno, - module, registry, NULL, NULL); - Py_END_CRITICAL_SECTION(); - Py_DECREF(message); - if (res != NULL) { - Py_DECREF(res); - ret = 0; - } + assert(st != NULL); + + Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex); + res = warn_explicit(tstate, category, message, filename, lineno, + module, registry, NULL, NULL); + Py_END_CRITICAL_SECTION(); + Py_DECREF(message); + if (res != NULL) { + Py_DECREF(res); + ret = 0; } } }