Mailing List Archive

gh-116664: Make module state Py_SETREF's in _warnings thread-safe (#116959)
https://github.com/python/cpython/commit/c1712ef066321c01bf09cba3f22fc474b5b8dfa7
commit: c1712ef066321c01bf09cba3f22fc474b5b8dfa7
branch: main
author: Erlend E. Aasland <erlend@python.org>
committer: erlend-aasland <erlend.aasland@protonmail.com>
date: 2024-03-28T15:05:08Z
summary:

gh-116664: Make module state Py_SETREF's in _warnings thread-safe (#116959)

Mark the swap operations as critical sections.

Add an internal Py_BEGIN_CRITICAL_SECTION_MUT API that takes a PyMutex
pointer instead of a PyObject pointer.

files:
M Include/internal/pycore_critical_section.h
M Include/internal/pycore_warnings.h
M Python/_warnings.c

diff --git a/Include/internal/pycore_critical_section.h b/Include/internal/pycore_critical_section.h
index 9163b5cf0f2e8a..23b85c2f9e9bb2 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 dfa82c569e1383..66a460e2a2c509 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_MUT()
#include "pycore_interp.h" // PyInterpreterState.warnings
#include "pycore_long.h" // _PyLong_GetZero()
#include "pycore_pyerrors.h" // _PyErr_Occurred()
@@ -235,14 +236,12 @@ get_warnings_attr(PyInterpreterState *interp, PyObject *attr, int try_import)
static PyObject *
get_once_registry(PyInterpreterState *interp)
{
- PyObject *registry;
-
WarningsState *st = warnings_get_state(interp);
- if (st == NULL) {
- return NULL;
- }
+ assert(st != NULL);
+
+ _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);

- registry = GET_WARNINGS_ATTR(interp, onceregistry, 0);
+ PyObject *registry = GET_WARNINGS_ATTR(interp, onceregistry, 0);
if (registry == NULL) {
if (PyErr_Occurred())
return NULL;
@@ -265,14 +264,12 @@ get_once_registry(PyInterpreterState *interp)
static PyObject *
get_default_action(PyInterpreterState *interp)
{
- PyObject *default_action;
-
WarningsState *st = warnings_get_state(interp);
- if (st == NULL) {
- return NULL;
- }
+ assert(st != NULL);

- default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0);
+ _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+
+ PyObject *default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0);
if (default_action == NULL) {
if (PyErr_Occurred()) {
return NULL;
@@ -299,15 +296,12 @@ 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;
WarningsState *st = warnings_get_state(interp);
- if (st == NULL) {
- return NULL;
- }
+ assert(st != NULL);

- warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0);
+ _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+
+ PyObject *warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0);
if (warnings_filters == NULL) {
if (PyErr_Occurred())
return NULL;
@@ -324,7 +318,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;
@@ -384,7 +378,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;
@@ -1000,8 +994,13 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level,
&filename, &lineno, &module, &registry))
return NULL;

+ WarningsState *st = warnings_get_state(tstate->interp);
+ assert(st != 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);
@@ -1149,8 +1148,14 @@ warnings_warn_explicit_impl(PyObject *module, PyObject *message,
return NULL;
}
}
+
+ WarningsState *st = warnings_get_state(tstate->interp);
+ assert(st != 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;
}
@@ -1290,8 +1295,14 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject *message,
if (tstate == NULL) {
return -1;
}
+
+ WarningsState *st = warnings_get_state(tstate->interp);
+ 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();
if (res == NULL)
return -1;
Py_DECREF(res);
@@ -1356,8 +1367,13 @@ PyErr_WarnExplicitFormat(PyObject *category,
PyObject *res;
PyThreadState *tstate = get_current_tstate();
if (tstate != NULL) {
+ WarningsState *st = warnings_get_state(tstate->interp);
+ 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);

_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-leave@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: list-python-checkins@lists.gossamer-threads.com