warnings.catch_warnings is async-unsafe #91505
Copy link
Copy link
Closed
Labels
stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
stdlibStandard Library Python modules in the Lib/ directoryStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
graingert commentedon Jun 28, 2022
See also #81785 and #50896
kumaraditya303 commentedon Sep 25, 2022
Using
contextvarsmakes sense here.gvanrossum commentedon Oct 16, 2022
Sounds like a big project.
joaoe commentedon Sep 12, 2023
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.
gvanrossum commentedon Sep 12, 2023
@joaoe Please describe your design and explain how it solves the problem. I'm very interested in hearing from you!
joaoe commentedon Sep 13, 2023
Hi
@gvanrossum surely :)
Here's a draft commit
joaoe@thread_safe_warnings
Notes:
catch_warnings()and activefilters, protected by aRLock. Usable by libraries that want to take over all warnings, like test frameworks andlogging.captureWarnings().contextvars, to fix the problem presented in the original post.catch_warnings()objects have been created and in which order. From the stack traversing downwards, one can find the current filters and who consumes messageslogging.captureWarnings(). If called with acatch_warnings()in context (this happens with pytest), it would break.Again, this is a draft, a suggestion.
Hopefully this helps bringing this problem forward.
gvanrossum commentedon Sep 13, 2023
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!
colesbury commentedon Oct 16, 2024
@timfel - we were discussing
warningsmodule 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!joaoe commentedon Oct 17, 2024
@colesbury May I join the conversation ? I made a draft implementation, obviously needs discussion and review.
colesbury commentedon Oct 17, 2024
@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:
warningsmodules thread-safe in a backwards compatible waycatch_warnings, like you do in your patchwarnings2module. @JelleZijlstra: Awarnings2module would be "not a great experience for users, and still has compatibility implications, since warnings can be raised from the core."decimalstyle "warnings context" to allow for a graceful transition to a contextvars-based solutiontimfel commentedon Oct 18, 2024
GraalPy just uses the warnings module as-is from CPython's stdlib, and implements the
_warningsmodule following the C impl closely. We haven't done anything to make it thread-safe, sorry.ngoldbaum commentedon Apr 14, 2025
@nascheme this is fixed now, isn't it? At least if you enable the config switch on the GIL-enabled build. Same with #128384.
nascheme commentedon Apr 14, 2025
Fixed by gh-130010. You have to set the
sys.flags.context_aware_warningsflag to ensure you get the new behavior.