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

Add thread-safe context manager to "warnings" module #128300

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Dec 27, 2024

The existing context manager warning.catch_warnings() is inherently thread-unsafe. Here is some example code that can show the issue:

from concurrent.futures import ThreadPoolExecutor

import warnings

def test_warning():
    with warnings.catch_warnings():
        warnings.simplefilter("ignore")
        warnings.warn("my warning", UserWarning)

tpe = ThreadPoolExecutor(max_workers=10)

threads = [tpe.submit(test_warning) for _ in range(100)]
[t.result() for t in threads]

Since test_warning() is running in separate threads, there is no consistent ordering of which context manager exits first. That means the filters value that gets restored is also not consistent (a race between the threads about which version of the filters value they will restore).

This PR adds a new thread-local (and async friendly) context manager that uses contextvars. The recommended pattern for new code that doesn't care about backwards compatibility would be:

with warnings.local_context() as ctx:
    ctx.simplefilter(...)
    ...

For code that wants to be compatible with older versions of Python, the suggested code is:

import warnings
try:
    warn_ctx = warnings.get_context
except AttributeError:
   def warn_ctx():
       return warnings

with warn_ctx().catch_warnings():
    warn_ctx().simplefilter(...)
   ...

with warn_ctx().catch_warnings(record=True) as log:
    ...

This change retains warnings.filters and warnings.catch_warnings() as mechanisms that use process global state. Conceptually, there will be two sets of warning filters, the legacy process global warnings.filters version and the thread local get_context()._filters version. Perhaps we could eventually warn when the process global versions are used but I think that would be many years in the future.

It would be intuitive if the thread local filtering was inherited when a new thread is created. I've created a separate PR that adds this feature to threading.Thread: gh-128209. That's a potentially controversial change but I think it's the correct thing to do. It will make contextvars behave similarly between asyncio tasks (which already inherit context) and threads. I think people will expect the warnings context manager to have lexical scope. If you see the code:

with warnings.local_context() as ctx:
    ctx.simplefilter(MyWarning)
    inner_function()

Then you would assume that inner_function() is going to have MyWarning filtered, even if it internally spawns a thread.

Regarding backwards compatibility, I did a code search and there are quite a few examples for code that manipulates warnings.filters directly, either mutating it or assigning a different list to the module global. There is also code that does this:

warnings.simplefilter(...)
...
warnings.filters.pop(0)

📚 Documentation preview 📚: https://cpython-previews--128300.org.readthedocs.build/

By default, inherit the context from the thread calling
`Thread.start()`.
Expose the mutex from _warnings.c and hold it when mutating the filter
state.
@nascheme nascheme force-pushed the warnings_local_context branch from 1f8b441 to 3952bb0 Compare December 28, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant