Skip to content

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

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Mar 28, 2024
Merged

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

Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
42e98b8
gh-116664: Make module state Py_SETREF's in _warnings thread-safe
erlend-aasland Mar 18, 2024
46dc0f4
Pull in main
erlend-aasland Mar 18, 2024
d222f7c
Protect warn_explicit() instead
erlend-aasland Mar 18, 2024
99e04de
Pull in main
erlend-aasland Mar 19, 2024
be08b22
Forgot one call
erlend-aasland Mar 19, 2024
3357659
Pull in main
erlend-aasland Mar 19, 2024
a87d84c
Use more granular critical sections
erlend-aasland Mar 19, 2024
30ee26d
Pull in main
erlend-aasland Mar 26, 2024
f9e549d
Put a mutex in _warnings_runtime_state
erlend-aasland Mar 27, 2024
4f014ab
Pull in main
erlend-aasland Mar 27, 2024
a6b127f
Pass a pointer
erlend-aasland Mar 27, 2024
d5d1ba3
Pull in main
erlend-aasland Mar 28, 2024
c6c1866
Address review: assert state and remove unneeded mutex initialization
erlend-aasland Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions Python/_warnings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
Expand Down Expand Up @@ -235,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;
Expand All @@ -265,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;
Expand All @@ -299,15 +300,14 @@ 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;
Expand All @@ -324,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;
Expand Down Expand Up @@ -384,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;
Expand Down Expand Up @@ -1000,8 +1000,10 @@ do_warn(PyObject *message, PyObject *category, Py_ssize_t stack_level,
&filename, &lineno, &module, &registry))
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);
Expand Down Expand Up @@ -1149,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;
}
Expand Down Expand Up @@ -1290,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);
Expand Down Expand Up @@ -1356,8 +1362,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);
Expand Down