GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-128384: Add locking to warnings.py. by nascheme · Pull Request #128386 · python/cpython

Conversation

@nascheme

@nascheme nascheme commented Dec 31, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

The warnings module has some (relatively minor) thread safety issues. The catch_warnings contex manager is a major issue but that will be handled in a different PR. There are races between updating the filters list and incrementing the _filters_version number. Also, the warn_explicit() uses the global state in a non-thread-safe way. These issues are relatively easy to fix with some extra locking.

Changes:

  • expose the mutex used by the _warnings module, as _acquire_lock and _release_lock
  • implement a lock context manager and lock module state as needed
  • fix a bug in DeprecatedTests that resulted in mixing py_warnings and c_warnings modules.

The mutex used is non-reentrant and so some care is required to avoid deadlocks. I restructured the code in warn_explicit() to reduce functions called within the locked section. Perhaps a re-entrant lock should be used instead?

  • Issue: warnings.catch_warnings is not thread safe or async safe #128384

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@bedevere-app bedevere-app Bot mentioned this pull request Dec 31, 2024
warnings.catch_warnings is not thread safe or async safe #128384
Closed
@nascheme
Add locking to warnings.py.
deb659d
Expose the mutex from _warnings.c and hold it when mutating the filters
list.
@nascheme
nascheme force-pushed the warnings_lock branch from a5ec58d to deb659d Compare December 31, 2024 23:24
@kumaraditya303

kumaraditya303 commented Jan 1, 2025

Copy link
Copy Markdown
Contributor

The mutex used is non-reentrant and so some care is required to avoid deadlocks. I restructured the code in warn_explicit() to reduce functions called within the locked section. Perhaps a re-entrant lock should be used instead?

I think the lock should be re-entrant, as locks are held when doing dict lookups which can call arbitrary python code.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 1, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 1, 2025
View reviewed changes
Comment thread Lib/warnings.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 1, 2025
View reviewed changes
Comment thread Lib/warnings.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

nascheme added 2 commits January 1, 2025 11:20
@nascheme
Rename function, add locking to _filters_mutated().
d5187b3
@nascheme
Use re-entrant lock for warnings module.
553d04f
kumaraditya303
kumaraditya303 reviewed Jan 2, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 2, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 2, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 2, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@nascheme
Fix locking for !Py_GIL_DISABLED build.
4b56ae6
@nascheme nascheme mentioned this pull request Jan 3, 2025
gh-128384: Use contextvar for catch_warnings() #128463
Closed
nascheme added 2 commits January 6, 2025 10:37
@nascheme
Add blurb.
6100ab2
@nascheme
Fix typo.
b9db71f
@nascheme
nascheme marked this pull request as ready for review January 6, 2025 18:42
@bedevere-app bedevere-app Bot added the awaiting core review label Jan 6, 2025
kumaraditya303
kumaraditya303 reviewed Jan 7, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V0xzwH.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.

@nascheme @kumaraditya303
Update Misc/NEWS.d/next/Library/2025-01-06-10-37-27.gh-issue-128384.V…
27e3acd
…0xzwH.rst

Co-authored-by: Kumar Aditya <[email protected]>
kumaraditya303
kumaraditya303 reviewed Jan 7, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

nascheme added 2 commits January 7, 2025 11:40
@nascheme
Merge branch 'main' into warnings_lock
dcdb699
@nascheme
Use locking for GIL build as well.
441b1ae
kumaraditya303
kumaraditya303 reviewed Jan 8, 2025
View reviewed changes
Comment thread Lib/warnings.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Jan 8, 2025
View reviewed changes
Comment thread Python/_warnings.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

@nascheme @kumaraditya303
Update Python/_warnings.c
9bd8875
Co-authored-by: Kumar Aditya <[email protected]>
@kumaraditya303

kumaraditya303 commented Jan 14, 2025

Copy link
Copy Markdown
Contributor

This looks good now from a brief look.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@nascheme
Hold lock while assigning self._module methods.
5b19978
@nascheme
nascheme merged commit 1c13c56 into python:main Jan 14, 2025
@bedevere-app bedevere-app Bot removed the awaiting core review label Jan 14, 2025
@devdanzin devdanzin mentioned this pull request Feb 7, 2025
Fatal Python error from warnings._release_lock() #129766
Closed
@sobolevn sobolevn mentioned this pull request Feb 7, 2025
gh-129766: Fix crash on calling warnings._release_lock with no lock #129771
Merged
@nascheme nascheme added the topic-free-threading label Apr 1, 2025
@krassowski krassowski mentioned this pull request Oct 7, 2026
os.fork() child can hang inside PyOS_AfterFork_Child() on the warnings lock (3.14 regression) #158973
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@nascheme @kumaraditya303
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.