Skip to content

Commit 1c13c56

Browse files
gh-128384: Add locking to warnings.py. (gh-128386)
Co-authored-by: Kumar Aditya <[email protected]>
1 parent d906bde commit 1c13c56

File tree

6 files changed

+237
-128
lines changed

6 files changed

+237
-128
lines changed

Include/internal/pycore_warnings.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ struct _warnings_runtime_state {
1414
PyObject *filters; /* List */
1515
PyObject *once_registry; /* Dict */
1616
PyObject *default_action; /* String */
17-
PyMutex mutex;
17+
_PyRecursiveMutex lock;
1818
long filters_version;
1919
};
2020

Lib/test/test_warnings/__init__.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -1521,7 +1521,7 @@ def test_late_resource_warning(self):
15211521
self.assertTrue(err.startswith(expected), ascii(err))
15221522

15231523

1524-
class DeprecatedTests(unittest.TestCase):
1524+
class DeprecatedTests(PyPublicAPITests):
15251525
def test_dunder_deprecated(self):
15261526
@deprecated("A will go away soon")
15271527
class A:

Lib/warnings.py

+109-79
Original file line numberDiff line numberDiff line change
@@ -185,24 +185,32 @@ def simplefilter(action, category=Warning, lineno=0, append=False):
185185
raise ValueError("lineno must be an int >= 0")
186186
_add_filter(action, None, category, None, lineno, append=append)
187187

188+
def _filters_mutated():
189+
# Even though this function is not part of the public API, it's used by
190+
# a fair amount of user code.
191+
with _lock:
192+
_filters_mutated_lock_held()
193+
188194
def _add_filter(*item, append):
189-
# Remove possible duplicate filters, so new one will be placed
190-
# in correct place. If append=True and duplicate exists, do nothing.
191-
if not append:
192-
try:
193-
filters.remove(item)
194-
except ValueError:
195-
pass
196-
filters.insert(0, item)
197-
else:
198-
if item not in filters:
199-
filters.append(item)
200-
_filters_mutated()
195+
with _lock:
196+
if not append:
197+
# Remove possible duplicate filters, so new one will be placed
198+
# in correct place. If append=True and duplicate exists, do nothing.
199+
try:
200+
filters.remove(item)
201+
except ValueError:
202+
pass
203+
filters.insert(0, item)
204+
else:
205+
if item not in filters:
206+
filters.append(item)
207+
_filters_mutated_lock_held()
201208

202209
def resetwarnings():
203210
"""Clear the list of warning filters, so that no filters are active."""
204-
filters[:] = []
205-
_filters_mutated()
211+
with _lock:
212+
filters[:] = []
213+
_filters_mutated_lock_held()
206214

207215
class _OptionError(Exception):
208216
"""Exception used by option processing helpers."""
@@ -353,64 +361,66 @@ def warn_explicit(message, category, filename, lineno,
353361
module = filename or "<unknown>"
354362
if module[-3:].lower() == ".py":
355363
module = module[:-3] # XXX What about leading pathname?
356-
if registry is None:
357-
registry = {}
358-
if registry.get('version', 0) != _filters_version:
359-
registry.clear()
360-
registry['version'] = _filters_version
361364
if isinstance(message, Warning):
362365
text = str(message)
363366
category = message.__class__
364367
else:
365368
text = message
366369
message = category(message)
367370
key = (text, category, lineno)
368-
# Quick test for common case
369-
if registry.get(key):
370-
return
371-
# Search the filters
372-
for item in filters:
373-
action, msg, cat, mod, ln = item
374-
if ((msg is None or msg.match(text)) and
375-
issubclass(category, cat) and
376-
(mod is None or mod.match(module)) and
377-
(ln == 0 or lineno == ln)):
378-
break
379-
else:
380-
action = defaultaction
381-
# Early exit actions
382-
if action == "ignore":
383-
return
371+
with _lock:
372+
if registry is None:
373+
registry = {}
374+
if registry.get('version', 0) != _filters_version:
375+
registry.clear()
376+
registry['version'] = _filters_version
377+
# Quick test for common case
378+
if registry.get(key):
379+
return
380+
# Search the filters
381+
for item in filters:
382+
action, msg, cat, mod, ln = item
383+
if ((msg is None or msg.match(text)) and
384+
issubclass(category, cat) and
385+
(mod is None or mod.match(module)) and
386+
(ln == 0 or lineno == ln)):
387+
break
388+
else:
389+
action = defaultaction
390+
# Early exit actions
391+
if action == "ignore":
392+
return
393+
394+
if action == "error":
395+
raise message
396+
# Other actions
397+
if action == "once":
398+
registry[key] = 1
399+
oncekey = (text, category)
400+
if onceregistry.get(oncekey):
401+
return
402+
onceregistry[oncekey] = 1
403+
elif action in {"always", "all"}:
404+
pass
405+
elif action == "module":
406+
registry[key] = 1
407+
altkey = (text, category, 0)
408+
if registry.get(altkey):
409+
return
410+
registry[altkey] = 1
411+
elif action == "default":
412+
registry[key] = 1
413+
else:
414+
# Unrecognized actions are errors
415+
raise RuntimeError(
416+
"Unrecognized action (%r) in warnings.filters:\n %s" %
417+
(action, item))
384418

385419
# Prime the linecache for formatting, in case the
386420
# "file" is actually in a zipfile or something.
387421
import linecache
388422
linecache.getlines(filename, module_globals)
389423

390-
if action == "error":
391-
raise message
392-
# Other actions
393-
if action == "once":
394-
registry[key] = 1
395-
oncekey = (text, category)
396-
if onceregistry.get(oncekey):
397-
return
398-
onceregistry[oncekey] = 1
399-
elif action in {"always", "all"}:
400-
pass
401-
elif action == "module":
402-
registry[key] = 1
403-
altkey = (text, category, 0)
404-
if registry.get(altkey):
405-
return
406-
registry[altkey] = 1
407-
elif action == "default":
408-
registry[key] = 1
409-
else:
410-
# Unrecognized actions are errors
411-
raise RuntimeError(
412-
"Unrecognized action (%r) in warnings.filters:\n %s" %
413-
(action, item))
414424
# Print message and context
415425
msg = WarningMessage(message, category, filename, lineno, source)
416426
_showwarnmsg(msg)
@@ -488,30 +498,32 @@ def __enter__(self):
488498
if self._entered:
489499
raise RuntimeError("Cannot enter %r twice" % self)
490500
self._entered = True
491-
self._filters = self._module.filters
492-
self._module.filters = self._filters[:]
493-
self._module._filters_mutated()
494-
self._showwarning = self._module.showwarning
495-
self._showwarnmsg_impl = self._module._showwarnmsg_impl
501+
with _lock:
502+
self._filters = self._module.filters
503+
self._module.filters = self._filters[:]
504+
self._module._filters_mutated_lock_held()
505+
self._showwarning = self._module.showwarning
506+
self._showwarnmsg_impl = self._module._showwarnmsg_impl
507+
if self._record:
508+
log = []
509+
self._module._showwarnmsg_impl = log.append
510+
# Reset showwarning() to the default implementation to make sure
511+
# that _showwarnmsg() calls _showwarnmsg_impl()
512+
self._module.showwarning = self._module._showwarning_orig
513+
else:
514+
log = None
496515
if self._filter is not None:
497516
simplefilter(*self._filter)
498-
if self._record:
499-
log = []
500-
self._module._showwarnmsg_impl = log.append
501-
# Reset showwarning() to the default implementation to make sure
502-
# that _showwarnmsg() calls _showwarnmsg_impl()
503-
self._module.showwarning = self._module._showwarning_orig
504-
return log
505-
else:
506-
return None
517+
return log
507518

508519
def __exit__(self, *exc_info):
509520
if not self._entered:
510521
raise RuntimeError("Cannot exit %r without entering first" % self)
511-
self._module.filters = self._filters
512-
self._module._filters_mutated()
513-
self._module.showwarning = self._showwarning
514-
self._module._showwarnmsg_impl = self._showwarnmsg_impl
522+
with _lock:
523+
self._module.filters = self._filters
524+
self._module._filters_mutated_lock_held()
525+
self._module.showwarning = self._showwarning
526+
self._module._showwarnmsg_impl = self._showwarnmsg_impl
515527

516528

517529
class deprecated:
@@ -701,18 +713,36 @@ def extract():
701713
# If either if the compiled regexs are None, match anything.
702714
try:
703715
from _warnings import (filters, _defaultaction, _onceregistry,
704-
warn, warn_explicit, _filters_mutated)
716+
warn, warn_explicit,
717+
_filters_mutated_lock_held,
718+
_acquire_lock, _release_lock,
719+
)
705720
defaultaction = _defaultaction
706721
onceregistry = _onceregistry
707722
_warnings_defaults = True
723+
724+
class _Lock:
725+
def __enter__(self):
726+
_acquire_lock()
727+
return self
728+
729+
def __exit__(self, *args):
730+
_release_lock()
731+
732+
_lock = _Lock()
733+
708734
except ImportError:
709735
filters = []
710736
defaultaction = "default"
711737
onceregistry = {}
712738

739+
import _thread
740+
741+
_lock = _thread.RLock()
742+
713743
_filters_version = 1
714744

715-
def _filters_mutated():
745+
def _filters_mutated_lock_held():
716746
global _filters_version
717747
_filters_version += 1
718748

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Add locking to :mod:`warnings` to avoid some data races when free-threading
2+
is used. Change ``_warnings_runtime_state.mutex`` to be a recursive mutex
3+
and expose it to :mod:`warnings`, via the :func:`!_acquire_lock` and
4+
:func:`!_release_lock` functions. The lock is held when ``filters`` and
5+
``_filters_version`` are updated.

0 commit comments

Comments
 (0)