Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from Mar 28, 2024
8 changes: 6 additions & 2 deletions Include/internal/pycore_critical_section.h
Expand Up @@ -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); \
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions Include/internal/pycore_warnings.h
Expand Up @@ -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;
};

Expand Down
58 changes: 37 additions & 21 deletions 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()
Expand Down Expand Up @@ -235,14 +236,12 @@
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;
Expand All @@ -265,14 +264,12 @@
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;
Expand All @@ -299,15 +296,12 @@
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;
Expand All @@ -324,7 +318,7 @@
}

/* 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;
Expand Down Expand Up @@ -384,7 +378,7 @@
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;
Expand Down Expand Up @@ -1000,8 +994,13 @@
&filename, &lineno, &module, &registry))
return NULL;

WarningsState *st = warnings_get_state(tstate->interp);

Check warning on line 997 in Python/_warnings.c

View workflow job for this annotation

GitHub Actions / Address sanitizer

unused variable ‘st’ [-Wunused-variable]
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);
Expand Down Expand Up @@ -1149,8 +1148,14 @@
return NULL;
}
}

WarningsState *st = warnings_get_state(tstate->interp);

Check warning on line 1152 in Python/_warnings.c

View workflow job for this annotation

GitHub Actions / Address sanitizer

unused variable ‘st’ [-Wunused-variable]
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;
}
Expand Down Expand Up @@ -1290,8 +1295,14 @@
if (tstate == NULL) {
return -1;
}

WarningsState *st = warnings_get_state(tstate->interp);

Check warning on line 1299 in Python/_warnings.c

View workflow job for this annotation

GitHub Actions / Address sanitizer

unused variable ‘st’ [-Wunused-variable]
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);
Expand Down Expand Up @@ -1356,8 +1367,13 @@
PyObject *res;
PyThreadState *tstate = get_current_tstate();
if (tstate != NULL) {
WarningsState *st = warnings_get_state(tstate->interp);

Check warning on line 1370 in Python/_warnings.c

View workflow job for this annotation

GitHub Actions / Address sanitizer

unused variable ‘st’ [-Wunused-variable]
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
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);
Expand Down