https://github.com/python/cpython/commit/1c13c56a34fc4c4d8969f0b6dc93d5208a50d61b
commit: 1c13c56a34fc4c4d8969f0b6dc93d5208a50d61b
branch: main
author: Neil Schemenauer <nas-git...@arctrix.com>
committer: nascheme <nas-git...@arctrix.com>
date: 2025-01-14T11:43:42-08:00
summary:
gh-128384: Add locking to warnings.py. (gh-128386)
Co-authored-by: Kumar Aditya <kumaradi...@python.org>
files:
A Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V0xzwH.rst
M Include/internal/pycore_warnings.h
M Lib/test/test_warnings/__init__.py
M Lib/warnings.py
M Python/_warnings.c
M Python/clinic/_warnings.c.h
diff --git a/Include/internal/pycore_warnings.h
b/Include/internal/pycore_warnings.h
index f9f6559312f4ef..672228cd6fbd19 100644
--- a/Include/internal/pycore_warnings.h
+++ b/Include/internal/pycore_warnings.h
@@ -14,7 +14,7 @@ struct _warnings_runtime_state {
PyObject *filters; /* List */
PyObject *once_registry; /* Dict */
PyObject *default_action; /* String */
- PyMutex mutex;
+ _PyRecursiveMutex lock;
long filters_version;
};
diff --git a/Lib/test/test_warnings/__init__.py
b/Lib/test/test_warnings/__init__.py
index 4e3c877896f295..4bd164b8a9a82b 100644
--- a/Lib/test/test_warnings/__init__.py
+++ b/Lib/test/test_warnings/__init__.py
@@ -1521,7 +1521,7 @@ def test_late_resource_warning(self):
self.assertTrue(err.startswith(expected), ascii(err))
-class DeprecatedTests(unittest.TestCase):
+class DeprecatedTests(PyPublicAPITests):
def test_dunder_deprecated(self):
@deprecated("A will go away soon")
class A:
diff --git a/Lib/warnings.py b/Lib/warnings.py
index e83cde37ab2d1a..f20b01372dd7a4 100644
--- a/Lib/warnings.py
+++ b/Lib/warnings.py
@@ -185,24 +185,32 @@ def simplefilter(action, category=Warning, lineno=0,
append=False):
raise ValueError("lineno must be an int >= 0")
_add_filter(action, None, category, None, lineno, append=append)
+def _filters_mutated():
+ # Even though this function is not part of the public API, it's used by
+ # a fair amount of user code.
+ with _lock:
+ _filters_mutated_lock_held()
+
def _add_filter(*item, append):
- # Remove possible duplicate filters, so new one will be placed
- # in correct place. If append=True and duplicate exists, do nothing.
- if not append:
- try:
- filters.remove(item)
- except ValueError:
- pass
- filters.insert(0, item)
- else:
- if item not in filters:
- filters.append(item)
- _filters_mutated()
+ with _lock:
+ if not append:
+ # Remove possible duplicate filters, so new one will be placed
+ # in correct place. If append=True and duplicate exists, do
nothing.
+ try:
+ filters.remove(item)
+ except ValueError:
+ pass
+ filters.insert(0, item)
+ else:
+ if item not in filters:
+ filters.append(item)
+ _filters_mutated_lock_held()
def resetwarnings():
"""Clear the list of warning filters, so that no filters are active."""
- filters[:] = []
- _filters_mutated()
+ with _lock:
+ filters[:] = []
+ _filters_mutated_lock_held()
class _OptionError(Exception):
"""Exception used by option processing helpers."""
@@ -353,11 +361,6 @@ def warn_explicit(message, category, filename, lineno,
module = filename or "<unknown>"
if module[-3:].lower() == ".py":
module = module[:-3] # XXX What about leading pathname?
- if registry is None:
- registry = {}
- if registry.get('version', 0) != _filters_version:
- registry.clear()
- registry['version'] = _filters_version
if isinstance(message, Warning):
text = str(message)
category = message.__class__
@@ -365,52 +368,59 @@ def warn_explicit(message, category, filename, lineno,
text = message
message = category(message)
key = (text, category, lineno)
- # Quick test for common case
- if registry.get(key):
- return
- # Search the filters
- for item in filters:
- action, msg, cat, mod, ln = item
- if ((msg is None or msg.match(text)) and
- issubclass(category, cat) and
- (mod is None or mod.match(module)) and
- (ln == 0 or lineno == ln)):
- break
- else:
- action = defaultaction
- # Early exit actions
- if action == "ignore":
- return
+ with _lock:
+ if registry is None:
+ registry = {}
+ if registry.get('version', 0) != _filters_version:
+ registry.clear()
+ registry['version'] = _filters_version
+ # Quick test for common case
+ if registry.get(key):
+ return
+ # Search the filters
+ for item in filters:
+ action, msg, cat, mod, ln = item
+ if ((msg is None or msg.match(text)) and
+ issubclass(category, cat) and
+ (mod is None or mod.match(module)) and
+ (ln == 0 or lineno == ln)):
+ break
+ else:
+ action = defaultaction
+ # Early exit actions
+ if action == "ignore":
+ return
+
+ if action == "error":
+ raise message
+ # Other actions
+ if action == "once":
+ registry[key] = 1
+ oncekey = (text, category)
+ if onceregistry.get(oncekey):
+ return
+ onceregistry[oncekey] = 1
+ elif action in {"always", "all"}:
+ pass
+ elif action == "module":
+ registry[key] = 1
+ altkey = (text, category, 0)
+ if registry.get(altkey):
+ return
+ registry[altkey] = 1
+ elif action == "default":
+ registry[key] = 1
+ else:
+ # Unrecognized actions are errors
+ raise RuntimeError(
+ "Unrecognized action (%r) in warnings.filters:\n %s" %
+ (action, item))
# Prime the linecache for formatting, in case the
# "file" is actually in a zipfile or something.
import linecache
linecache.getlines(filename, module_globals)
- if action == "error":
- raise message
- # Other actions
- if action == "once":
- registry[key] = 1
- oncekey = (text, category)
- if onceregistry.get(oncekey):
- return
- onceregistry[oncekey] = 1
- elif action in {"always", "all"}:
- pass
- elif action == "module":
- registry[key] = 1
- altkey = (text, category, 0)
- if registry.get(altkey):
- return
- registry[altkey] = 1
- elif action == "default":
- registry[key] = 1
- else:
- # Unrecognized actions are errors
- raise RuntimeError(
- "Unrecognized action (%r) in warnings.filters:\n %s" %
- (action, item))
# Print message and context
msg = WarningMessage(message, category, filename, lineno, source)
_showwarnmsg(msg)
@@ -488,30 +498,32 @@ def __enter__(self):
if self._entered:
raise RuntimeError("Cannot enter %r twice" % self)
self._entered = True
- self._filters = self._module.filters
- self._module.filters = self._filters[:]
- self._module._filters_mutated()
- self._showwarning = self._module.showwarning
- self._showwarnmsg_impl = self._module._showwarnmsg_impl
+ with _lock:
+ self._filters = self._module.filters
+ self._module.filters = self._filters[:]
+ self._module._filters_mutated_lock_held()
+ self._showwarning = self._module.showwarning
+ self._showwarnmsg_impl = self._module._showwarnmsg_impl
+ if self._record:
+ log = []
+ self._module._showwarnmsg_impl = log.append
+ # Reset showwarning() to the default implementation to make
sure
+ # that _showwarnmsg() calls _showwarnmsg_impl()
+ self._module.showwarning = self._module._showwarning_orig
+ else:
+ log = None
if self._filter is not None:
simplefilter(*self._filter)
- if self._record:
- log = []
- self._module._showwarnmsg_impl = log.append
- # Reset showwarning() to the default implementation to make sure
- # that _showwarnmsg() calls _showwarnmsg_impl()
- self._module.showwarning = self._module._showwarning_orig
- return log
- else:
- return None
+ return log
def __exit__(self, *exc_info):
if not self._entered:
raise RuntimeError("Cannot exit %r without entering first" % self)
- self._module.filters = self._filters
- self._module._filters_mutated()
- self._module.showwarning = self._showwarning
- self._module._showwarnmsg_impl = self._showwarnmsg_impl
+ with _lock:
+ self._module.filters = self._filters
+ self._module._filters_mutated_lock_held()
+ self._module.showwarning = self._showwarning
+ self._module._showwarnmsg_impl = self._showwarnmsg_impl
class deprecated:
@@ -701,18 +713,36 @@ def extract():
# If either if the compiled regexs are None, match anything.
try:
from _warnings import (filters, _defaultaction, _onceregistry,
- warn, warn_explicit, _filters_mutated)
+ warn, warn_explicit,
+ _filters_mutated_lock_held,
+ _acquire_lock, _release_lock,
+ )
defaultaction = _defaultaction
onceregistry = _onceregistry
_warnings_defaults = True
+
+ class _Lock:
+ def __enter__(self):
+ _acquire_lock()
+ return self
+
+ def __exit__(self, *args):
+ _release_lock()
+
+ _lock = _Lock()
+
except ImportError:
filters = []
defaultaction = "default"
onceregistry = {}
+ import _thread
+
+ _lock = _thread.RLock()
+
_filters_version = 1
- def _filters_mutated():
+ def _filters_mutated_lock_held():
global _filters_version
_filters_version += 1
diff --git
a/Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V0xzwH.rst
b/Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V0xzwH.rst
new file mode 100644
index 00000000000000..2ca592be20b681
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V0xzwH.rst
@@ -0,0 +1,5 @@
+Add locking to :mod:`warnings` to avoid some data races when free-threading
+is used. Change ``_warnings_runtime_state.mutex`` to be a recursive mutex
+and expose it to :mod:`warnings`, via the :func:`!_acquire_lock` and
+:func:`!_release_lock` functions. The lock is held when ``filters`` and
+``_filters_version`` are updated.
diff --git a/Python/_warnings.c b/Python/_warnings.c
index e05ba99e8eaec4..283f203c72c9bf 100644
--- a/Python/_warnings.c
+++ b/Python/_warnings.c
@@ -232,6 +232,61 @@ get_warnings_attr(PyInterpreterState *interp, PyObject
*attr, int try_import)
return obj;
}
+static inline void
+warnings_lock(PyInterpreterState *interp)
+{
+ WarningsState *st = warnings_get_state(interp);
+ assert(st != NULL);
+ _PyRecursiveMutex_Lock(&st->lock);
+}
+
+static inline void
+warnings_unlock(PyInterpreterState *interp)
+{
+ WarningsState *st = warnings_get_state(interp);
+ assert(st != NULL);
+ _PyRecursiveMutex_Unlock(&st->lock);
+}
+
+static inline bool
+warnings_lock_held(WarningsState *st)
+{
+ return PyMutex_IsLocked(&st->lock.mutex);
+}
+
+/*[clinic input]
+_acquire_lock as warnings_acquire_lock
+
+[clinic start generated code]*/
+
+static PyObject *
+warnings_acquire_lock_impl(PyObject *module)
+/*[clinic end generated code: output=594313457d1bf8e1 input=46ec20e55acca52f]*/
+{
+ PyInterpreterState *interp = get_current_interp();
+ if (interp == NULL) {
+ return NULL;
+ }
+ warnings_lock(interp);
+ Py_RETURN_NONE;
+}
+
+/*[clinic input]
+_release_lock as warnings_release_lock
+
+[clinic start generated code]*/
+
+static PyObject *
+warnings_release_lock_impl(PyObject *module)
+/*[clinic end generated code: output=d73d5a8789396750 input=ea01bb77870c5693]*/
+{
+ PyInterpreterState *interp = get_current_interp();
+ if (interp == NULL) {
+ return NULL;
+ }
+ warnings_unlock(interp);
+ Py_RETURN_NONE;
+}
static PyObject *
get_once_registry(PyInterpreterState *interp)
@@ -239,7 +294,7 @@ get_once_registry(PyInterpreterState *interp)
WarningsState *st = warnings_get_state(interp);
assert(st != NULL);
- _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+ assert(warnings_lock_held(st));
PyObject *registry = GET_WARNINGS_ATTR(interp, onceregistry, 0);
if (registry == NULL) {
@@ -267,7 +322,7 @@ get_default_action(PyInterpreterState *interp)
WarningsState *st = warnings_get_state(interp);
assert(st != NULL);
- _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+ assert(warnings_lock_held(st));
PyObject *default_action = GET_WARNINGS_ATTR(interp, defaultaction, 0);
if (default_action == NULL) {
@@ -299,7 +354,7 @@ get_filter(PyInterpreterState *interp, PyObject *category,
WarningsState *st = warnings_get_state(interp);
assert(st != NULL);
- _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+ assert(warnings_lock_held(st));
PyObject *warnings_filters = GET_WARNINGS_ATTR(interp, filters, 0);
if (warnings_filters == NULL) {
@@ -399,7 +454,7 @@ already_warned(PyInterpreterState *interp, PyObject
*registry, PyObject *key,
WarningsState *st = warnings_get_state(interp);
assert(st != NULL);
- _Py_CRITICAL_SECTION_ASSERT_MUTEX_LOCKED(&st->mutex);
+ assert(warnings_lock_held(st));
PyObject *version_obj;
if (PyDict_GetItemRef(registry, &_Py_ID(version), &version_obj) < 0) {
@@ -994,15 +1049,10 @@ do_warn(PyObject *message, PyObject *category,
Py_ssize_t stack_level,
&filename, &lineno, &module, ®istry))
return NULL;
-#ifdef Py_GIL_DISABLED
- WarningsState *st = warnings_get_state(tstate->interp);
- assert(st != NULL);
-#endif
-
- Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
+ warnings_lock(tstate->interp);
res = warn_explicit(tstate, category, message, filename, lineno, module,
registry,
NULL, source);
- Py_END_CRITICAL_SECTION();
+ warnings_unlock(tstate->interp);
Py_DECREF(filename);
Py_DECREF(registry);
Py_DECREF(module);
@@ -1151,27 +1201,22 @@ warnings_warn_explicit_impl(PyObject *module, PyObject
*message,
}
}
-#ifdef Py_GIL_DISABLED
- WarningsState *st = warnings_get_state(tstate->interp);
- assert(st != NULL);
-#endif
-
- Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
+ warnings_lock(tstate->interp);
returned = warn_explicit(tstate, category, message, filename, lineno,
mod, registry, source_line, sourceobj);
- Py_END_CRITICAL_SECTION();
+ warnings_unlock(tstate->interp);
Py_XDECREF(source_line);
return returned;
}
/*[clinic input]
-_filters_mutated as warnings_filters_mutated
+_filters_mutated_lock_held as warnings_filters_mutated_lock_held
[clinic start generated code]*/
static PyObject *
-warnings_filters_mutated_impl(PyObject *module)
-/*[clinic end generated code: output=8ce517abd12b88f4 input=35ecbf08ee2491b2]*/
+warnings_filters_mutated_lock_held_impl(PyObject *module)
+/*[clinic end generated code: output=df5c84f044e856ec input=34208bf03d70e432]*/
{
PyInterpreterState *interp = get_current_interp();
if (interp == NULL) {
@@ -1181,14 +1226,17 @@ warnings_filters_mutated_impl(PyObject *module)
WarningsState *st = warnings_get_state(interp);
assert(st != NULL);
- Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
+ // Note that the lock must be held by the caller.
+ if (!warnings_lock_held(st)) {
+ PyErr_SetString(PyExc_RuntimeError, "warnings lock is not held");
+ return NULL;
+ }
+
st->filters_version++;
- Py_END_CRITICAL_SECTION();
Py_RETURN_NONE;
}
-
/* Function to issue a warning message; may raise an exception. */
static int
@@ -1303,15 +1351,10 @@ PyErr_WarnExplicitObject(PyObject *category, PyObject
*message,
return -1;
}
-#ifdef Py_GIL_DISABLED
- WarningsState *st = warnings_get_state(tstate->interp);
- assert(st != NULL);
-#endif
-
- Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
+ warnings_lock(tstate->interp);
res = warn_explicit(tstate, category, message, filename, lineno,
module, registry, NULL, NULL);
- Py_END_CRITICAL_SECTION();
+ warnings_unlock(tstate->interp);
if (res == NULL)
return -1;
Py_DECREF(res);
@@ -1376,15 +1419,10 @@ PyErr_WarnExplicitFormat(PyObject *category,
PyObject *res;
PyThreadState *tstate = get_current_tstate();
if (tstate != NULL) {
-#ifdef Py_GIL_DISABLED
- WarningsState *st = warnings_get_state(tstate->interp);
- assert(st != NULL);
-#endif
-
- Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
+ warnings_lock(tstate->interp);
res = warn_explicit(tstate, category, message, filename, lineno,
module, registry, NULL, NULL);
- Py_END_CRITICAL_SECTION();
+ warnings_unlock(tstate->interp);
Py_DECREF(message);
if (res != NULL) {
Py_DECREF(res);
@@ -1464,7 +1502,9 @@ _PyErr_WarnUnawaitedCoroutine(PyObject *coro)
static PyMethodDef warnings_functions[] = {
WARNINGS_WARN_METHODDEF
WARNINGS_WARN_EXPLICIT_METHODDEF
- WARNINGS_FILTERS_MUTATED_METHODDEF
+ WARNINGS_FILTERS_MUTATED_LOCK_HELD_METHODDEF
+ WARNINGS_ACQUIRE_LOCK_METHODDEF
+ WARNINGS_RELEASE_LOCK_METHODDEF
/* XXX(brett.cannon): add showwarning? */
/* XXX(brett.cannon): Reasonable to add formatwarning? */
{NULL, NULL} /* sentinel */
diff --git a/Python/clinic/_warnings.c.h b/Python/clinic/_warnings.c.h
index 9a2c33f2ea8169..bcb4b344fa4370 100644
--- a/Python/clinic/_warnings.c.h
+++ b/Python/clinic/_warnings.c.h
@@ -9,6 +9,40 @@ preserve
#include "pycore_abstract.h" // _PyNumber_Index()
#include "pycore_modsupport.h" // _PyArg_UnpackKeywords()
+PyDoc_STRVAR(warnings_acquire_lock__doc__,
+"_acquire_lock($module, /)\n"
+"--\n"
+"\n");
+
+#define WARNINGS_ACQUIRE_LOCK_METHODDEF \
+ {"_acquire_lock", (PyCFunction)warnings_acquire_lock, METH_NOARGS,
warnings_acquire_lock__doc__},
+
+static PyObject *
+warnings_acquire_lock_impl(PyObject *module);
+
+static PyObject *
+warnings_acquire_lock(PyObject *module, PyObject *Py_UNUSED(ignored))
+{
+ return warnings_acquire_lock_impl(module);
+}
+
+PyDoc_STRVAR(warnings_release_lock__doc__,
+"_release_lock($module, /)\n"
+"--\n"
+"\n");
+
+#define WARNINGS_RELEASE_LOCK_METHODDEF \
+ {"_release_lock", (PyCFunction)warnings_release_lock, METH_NOARGS,
warnings_release_lock__doc__},
+
+static PyObject *
+warnings_release_lock_impl(PyObject *module);
+
+static PyObject *
+warnings_release_lock(PyObject *module, PyObject *Py_UNUSED(ignored))
+{
+ return warnings_release_lock_impl(module);
+}
+
PyDoc_STRVAR(warnings_warn__doc__,
"warn($module, /, message, category=None, stacklevel=1, source=None, *,\n"
" skip_file_prefixes=<unrepresentable>)\n"
@@ -230,20 +264,20 @@ warnings_warn_explicit(PyObject *module, PyObject *const
*args, Py_ssize_t nargs
return return_value;
}
-PyDoc_STRVAR(warnings_filters_mutated__doc__,
-"_filters_mutated($module, /)\n"
+PyDoc_STRVAR(warnings_filters_mutated_lock_held__doc__,
+"_filters_mutated_lock_held($module, /)\n"
"--\n"
"\n");
-#define WARNINGS_FILTERS_MUTATED_METHODDEF \
- {"_filters_mutated", (PyCFunction)warnings_filters_mutated, METH_NOARGS,
warnings_filters_mutated__doc__},
+#define WARNINGS_FILTERS_MUTATED_LOCK_HELD_METHODDEF \
+ {"_filters_mutated_lock_held",
(PyCFunction)warnings_filters_mutated_lock_held, METH_NOARGS,
warnings_filters_mutated_lock_held__doc__},
static PyObject *
-warnings_filters_mutated_impl(PyObject *module);
+warnings_filters_mutated_lock_held_impl(PyObject *module);
static PyObject *
-warnings_filters_mutated(PyObject *module, PyObject *Py_UNUSED(ignored))
+warnings_filters_mutated_lock_held(PyObject *module, PyObject
*Py_UNUSED(ignored))
{
- return warnings_filters_mutated_impl(module);
+ return warnings_filters_mutated_lock_held_impl(module);
}
-/*[clinic end generated code: output=ed02c0f521a03a37 input=a9049054013a1b77]*/
+/*[clinic end generated code: output=d9d32a8b59a30683 input=a9049054013a1b77]*/
_______________________________________________
Python-checkins mailing list -- python-checkins@python.org
To unsubscribe send an email to python-checkins-le...@python.org
https://mail.python.org/mailman3/lists/python-checkins.python.org/
Member address: arch...@mail-archive.com
