Skip to content

warnings.catch_warnings is async-unsafe #91505

Description

@fstirlitz
import warnings
import asyncio

async def spam():
	with warnings.catch_warnings(record=True) as ws:
		await asyncio.sleep(0.1)
		w = Warning("12345")
		warnings.warn(w)
		assert w in (ww.message for ww in ws)

async def ham():
	with warnings.catch_warnings(record=True) as ws:
		await asyncio.sleep(0.2)

async def main():
	await asyncio.gather(spam(), ham())

asyncio.run(main())

Despite technically being documented (https://docs.python.org/3/library/warnings.html#warnings.catch_warnings), this is very error-prone, surprising and not at all useful. The warnings module should probably use contextvars to manage the warning handler.

Activity

  1. added
    type-bugAn unexpected behavior, bug, or error
    stdlibStandard Library Python modules in the Lib/ directory
    on Apr 13, 2022
  2. graingert commented on Jun 28, 2022

    @graingert
    Contributor

    See also #81785 and #50896

  3. kumaraditya303 commented on Sep 25, 2022

    @kumaraditya303
    Contributor

    Using contextvars makes sense here.

  4. gvanrossum commented on Oct 16, 2022

    @gvanrossum
    Member

    Sounds like a big project.

  5. joaoe commented on Sep 12, 2023

    @joaoe

    Hi.
    I've looked into the warnings module code.
    I so happens that in my project warnings, logging and pytest don't cooperate too well.

    The warning module stores pointers to the original warning functions and swaps that function in and out with the catch_warnings() context mnager. This is obviously a recipe for disaster either running with threads or just if mixing catch_warnings() with other APIs that also like to take over the warnings function.

    My suggestion is that all monkeypatching of global functions in the warnings module must be removed. I have a draft implementation I can push eventually, which uses contextvars to keep track of which catch_warnings() object is active and push the warning messages there. And that's it is. So far my branch is incomplete because it also needs to keep track of filters activated with catch_warnings().

    Hopefully we can have a discussion about the implementation and potentially consider including the new implentation in the next major release.

  6. gvanrossum commented on Sep 12, 2023

    @gvanrossum
    Member

    @joaoe Please describe your design and explain how it solves the problem. I'm very interested in hearing from you!

  7. joaoe commented on Sep 13, 2023

    @joaoe

    Hi
    @gvanrossum surely :)

    Here's a draft commit
    joaoe@thread_safe_warnings
    Notes:

    • there is a global stack of catch_warnings() and active filters, protected by a RLock. Usable by libraries that want to take over all warnings, like test frameworks and logging.captureWarnings().
    • there is also a similar thread local stack using contextvars, to fix the problem presented in the original post.
    • the stack keeps track of which catch_warnings() objects have been created and in which order. From the stack traversing downwards, one can find the current filters and who consumes messages
    • the commit is a compromise between keeping some old behavior and adding new one, e.g., still having a global filters list.
    • with this implentation, global symbols in the warnings module, like showwarning and filters become "read-only" meaning, it shouldn't actualy be supported to swap these objects in and out to control the behavior of the module. This is implemented with the Protect object in the end, but a final implementation would most likely have some different implementation, e.g., printing a deprecation warning?
    • the commit also fixes logging.captureWarnings(). If called with a catch_warnings() in context (this happens with pytest), it would break.
    • the commit touches no C code although that would need to be fixed as well
    • I fixed a number of tests. Many other tests fail for me, but if I extract some tests to the side and run the code separately, it works. Some shenanigans with the test framework perhaps or my environment setup. Skipping "import _warnings" fixed some tests and broke others.

    Again, this is a draft, a suggestion.
    Hopefully this helps bringing this problem forward.

  8. gvanrossum commented on Sep 13, 2023

    @gvanrossum
    Member

    Hi @joaoe, I'm afraid I won't personally have time to review this any time soon. I hope that another core dev or triager might be interested. Maybe @kumaraditya303, @serhiy-storchaka, or @sobolevn ? Also, I recommend that you open a Draft PR -- that's easier for reviewers. The title of the draft PR should start with "gh-91505:" so it will be linked back to this issue. Good luck!

  9. colesbury commented on Oct 16, 2024

    @colesbury
    Contributor

    @timfel - we were discussing warnings module thread-safety and async-safety issues in the CPython Discord and @cfbolz mentioned that you may have done related work in GraalPy. I was wondering if you have insights into particular approaches. Thanks!

  10. joaoe commented on Oct 17, 2024

    @joaoe

    @colesbury May I join the conversation ? I made a draft implementation, obviously needs discussion and review.

  11. colesbury commented on Oct 17, 2024

    @colesbury
    Contributor

    @joaoe, yes that's why I posted here so that we can continue the discussion publicly.

    I don't think the discussion on Discord covered much new ground:

    • It's not obvious how to make the warnings modules thread-safe in a backwards compatible way
    • One approach is to add a new argument to catch_warnings, like you do in your patch
    • @ngoldbaum raised the idea of a warnings2 module. @JelleZijlstra: A warnings2 module would be "not a great experience for users, and still has compatibility implications, since warnings can be raised from the core."
    • @ncoghlan suggested a decimal style "warnings context" to allow for a graceful transition to a contextvars-based solution
  12. timfel commented on Oct 18, 2024

    @timfel

    @timfel - we were discussing warnings module thread-safety and async-safety issues in the CPython Discord and @cfbolz mentioned that you may have done related work in GraalPy. I was wondering if you have insights into particular approaches. Thanks!

    GraalPy just uses the warnings module as-is from CPython's stdlib, and implements the _warnings module following the C impl closely. We haven't done anything to make it thread-safe, sorry.

  13. ngoldbaum commented on Apr 14, 2025

    @ngoldbaum
    Contributor

    @nascheme this is fixed now, isn't it? At least if you enable the config switch on the GIL-enabled build. Same with #128384.

  14. nascheme commented on Apr 14, 2025

    @nascheme
    Member

    Fixed by gh-130010. You have to set the sys.flags.context_aware_warnings flag to ensure you get the new behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    stdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions