gh-128384: Add locking to warnings.py. (gh-128386)

Co-authored-by: Kumar Aditya <kumaraditya@python.org>

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;
 };
 

Lib/test/test_warnings/__init__.py

@@ -1521,7 +1521,7 @@ a=A()
         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:

Lib/warnings.py

@@ -185,10 +185,17 @@ 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):
+    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.
-    if not append:
             try:
                 filters.remove(item)
             except ValueError:
@@ -197,12 +204,13 @@ def _add_filter(*item, append):
         else:
             if item not in filters:
                 filters.append(item)
-    _filters_mutated()
+        _filters_mutated_lock_held()
 
 def resetwarnings():
     """Clear the list of warning filters, so that no filters are active."""
+    with _lock:
         filters[:] = []
-    _filters_mutated()
+        _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,6 +368,12 @@ def warn_explicit(message, category, filename, lineno,
         text = message
         message = category(message)
     key = (text, category, lineno)
+    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
@@ -382,11 +391,6 @@ def warn_explicit(message, category, filename, lineno,
         if action == "ignore":
             return
 
-    # 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
@@ -411,6 +415,12 @@ def warn_explicit(message, category, filename, lineno,
             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)
+
     # Print message and context
     msg = WarningMessage(message, category, filename, lineno, source)
     _showwarnmsg(msg)
@@ -488,28 +498,30 @@ class catch_warnings(object):
         if self._entered:
             raise RuntimeError("Cannot enter %r twice" % self)
         self._entered = True
+        with _lock:
             self._filters = self._module.filters
             self._module.filters = self._filters[:]
-        self._module._filters_mutated()
+            self._module._filters_mutated_lock_held()
             self._showwarning = self._module.showwarning
             self._showwarnmsg_impl = self._module._showwarnmsg_impl
-        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
+                log = None
+        if self._filter is not None:
+            simplefilter(*self._filter)
+        return log
 
     def __exit__(self, *exc_info):
         if not self._entered:
             raise RuntimeError("Cannot exit %r without entering first" % self)
+        with _lock:
             self._module.filters = self._filters
-        self._module._filters_mutated()
+            self._module._filters_mutated_lock_held()
             self._module.showwarning = self._showwarning
             self._module._showwarnmsg_impl = self._showwarnmsg_impl
 
@@ -701,18 +713,36 @@ def _warn_unawaited_coroutine(coro):
 # 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
 

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.

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, &registry))
         return NULL;
 
-#ifdef Py_GIL_DISABLED
+    warnings_lock(tstate->interp);
-    WarningsState *st = warnings_get_state(tstate->interp);
-    assert(st != NULL);
-#endif
-
-    Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
     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
+    warnings_lock(tstate->interp);
-    WarningsState *st = warnings_get_state(tstate->interp);
-    assert(st != NULL);
-#endif
-
-    Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
     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)
+warnings_filters_mutated_lock_held_impl(PyObject *module)
-/*[clinic end generated code: output=8ce517abd12b88f4 input=35ecbf08ee2491b2]*/
+/*[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
+    warnings_lock(tstate->interp);
-    WarningsState *st = warnings_get_state(tstate->interp);
-    assert(st != NULL);
-#endif
-
-    Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
     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
+            warnings_lock(tstate->interp);
-            WarningsState *st = warnings_get_state(tstate->interp);
-            assert(st != NULL);
-#endif
-
-            Py_BEGIN_CRITICAL_SECTION_MUT(&st->mutex);
             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 */

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 @@ exit:
     return return_value;
 }
 
-PyDoc_STRVAR(warnings_filters_mutated__doc__,
+PyDoc_STRVAR(warnings_filters_mutated_lock_held__doc__,
-"_filters_mutated($module, /)\n"
+"_filters_mutated_lock_held($module, /)\n"
 "--\n"
 "\n");
 
-#define WARNINGS_FILTERS_MUTATED_METHODDEF    \
+#define WARNINGS_FILTERS_MUTATED_LOCK_HELD_METHODDEF    \
-    {"_filters_mutated", (PyCFunction)warnings_filters_mutated, METH_NOARGS, warnings_filters_mutated__doc__},
+    {"_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]*/