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-141148: ensure tasks/threads get fresh copy of decimal.Context by nascheme · Pull Request #146482 · python/cpython

Conversation

@nascheme

@nascheme nascheme commented Mar 26, 2026 •
edited by github-actions Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Ensure that decimal.getcontext() returns a per-task copy of the decimal.Context so that mutations are isolated between asyncio tasks and threads when sys.flags.thread_inherit_context is set.

This change is required because decimal.Context instances are mutable. The contextvars.Context object uses an immutable data structure (HAMT) to store variable bindings. That ensures each task has its own set of contextvar bindings. However, it doesn't help if the contextvar value itself is mutated.

To fix this bug, the ContextVar.get_changed() method was added. This allows the decimal module to check if the contextvar value is newly set in this task/thread or if it is a value inherited. In the latter case, we copy the decimal.Context object to ensure that potential mutations to it are isolated.

There is a downside to this change. The contextvars.Context object has gained an additional pointer value PyHamtObject *ctx_vars_origin. I think the memory overhead of that pointer itself is negligible. That reference also keeps the entries in the "origin" HAMT object alive for as long as the new context exists. That could keep some contextvar values alive for longer.

I think in practice this is okay as well. Generally the contextvar values should be small objects. Also, if the task/thread that calls Context.run() is still around then the ctx_vars_origin object is likely still alive anyhow, due to that task holding a reference to it. It is only if the original task calls ContextVar.set() that you might free something.

  • Issue: data races in decimal module with global context #141148

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
nascheme added 2 commits March 25, 2026 18:08
@nascheme
Add PyContextVar_GetChanged() method.
0a20e79
@nascheme
Use ContextVar.get_changed() for decimal context.
b33469e 
Ensure that decimal.getcontext() returns a per-task copy of the
decimal.Context() so that mutations are isolated between async tasks
and threads using sys.flags.thread_inherit_context.
@bedevere-app bedevere-app Bot mentioned this pull request Mar 26, 2026
data races in decimal module with global context #141148
Open
@nascheme
Mark unit test that needs working threads.
74482e3
@skirpichev

skirpichev commented Mar 26, 2026

Copy link
Copy Markdown
Member

I think it's a correct approach.

This introduce new C function, shouldn't it be discussed first with the C-API WG?
All reactions

Sorry, something went wrong.

Uh oh!

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

nascheme commented Mar 27, 2026

Copy link
Copy Markdown
Member Author

This introduce new C function, shouldn't it be discussed first with the C-API WG?

Yes, I think so. We could make APIs private so that only the decimal module could use them. However, I think a pattern where you have contextvar values that you copy-on-update would be useful (the get_changed() method enables this). Otherwise, you have to "flatten" your data structure so that all of the bindings are inside the contextvars.Context() object. Or I suppose you can make those values immutable and re-bind a new one when you update.
All reactions

Sorry, something went wrong.

Uh oh!

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

skirpichev commented Mar 29, 2026

Copy link
Copy Markdown
Member

However, I think a pattern where you have contextvar values that you copy-on-update would be useful (the get_changed() method enables this).

Yes, I suspect, that gmpy2 was affected as well: https://github.com/gmpy2/gmpy2/blob/194a6dba7c3f497eebe1b3ffa234addc15f2c7c1/src/gmpy2_context.c

I think that same can be made without new API, using several contextvars.Context() objects. But I doubt about efficiency of such alternative.

Otherwise, you have to "flatten" your data structure so that all of the bindings are inside the contextvars.Context() object.

I don't think it's possible without a huge compatibility break.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
kumaraditya303
kumaraditya303 reviewed Apr 5, 2026
View reviewed changes
Comment thread Python/context.c
return 1;
}

return cur_val != orig_val;

@kumaraditya303 kumaraditya303 Apr 5, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the comparison by identity safe and correct here or should it call rich compare on the objects?

Sorry, something went wrong.

Uh oh!

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

@nascheme nascheme Apr 5, 2026

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe identity is what we want here.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@nascheme
nascheme marked this pull request as ready for review April 9, 2026 15:34
@nascheme
nascheme requested a review from 1st1 as a code owner April 9, 2026 15:34
@bedevere-app bedevere-app Bot added the awaiting core review label Apr 9, 2026
@devdanzin devdanzin mentioned this pull request Apr 29, 2026
_decimal: mpd_context_t::status/traps mutated non-atomically leading to data race #149142
Open
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 30 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label May 18, 2026
@LindaSummer LindaSummer mentioned this pull request May 30, 2026
GH-149142: Fix decimal mpd_context_t.status race in free-threading #150598
Open
nascheme added 2 commits June 17, 2026 16:44
@nascheme
Remove public PyContextVar_GetChanged() API.
c2dbdca 
Use a private API instead (ctx._get_changed() is the Python-level API).
@nascheme
Merge remote-tracking branch 'origin/main' into decimal_ctx_inherit
5835fac
@read-the-docs-community

read-the-docs-community Bot commented Jun 17, 2026

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33191341 | 📁 Comparing c2dbdca against main (a8d74c0)

  🔍 Preview build  

255 files changed · ± 248 modified · - 7 deleted

± Modified

  • bugs.html
  • glossary.html
  • index.html
  • c-api/allocation.html
  • c-api/buffer.html
  • c-api/bytearray.html
  • c-api/bytes.html
  • c-api/capsule.html
  • c-api/code.html
  • c-api/complex.html
  • and 238 more...

- Deleted

  • c-api/sentinel.html
  • c-api/slots.html
  • deprecations/pending-removal-in-3.21.html
  • deprecations/soft-deprecations.html
  • howto/abi3t-migration.html
  • library/asyncio-threading.html
  • whatsnew/3.16.html
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nascheme
Merge remote-tracking branch 'origin/main' into decimal_ctx_inherit
a7dacfa
@nascheme

nascheme commented Jun 22, 2026

Copy link
Copy Markdown
Member Author

I revised this PR so it no longer adds a new public C API. I think there is little need to have get_changed() as a public API. That method is useful to make the decimal context more thread-safe. However, having a mutable object bound to the context variable is not a pattern we should encourage.

Another concern about this is that it will keep the previous context vars alive for longer. For example, if a context variable is bound to a large object and then you re-bind it, the old object is keep alive via the ctx_vars_origin reference. Or, perhaps people expect a weakref callback or del method to run when the context var is re-bound.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nascheme nascheme mentioned this pull request Jun 22, 2026
GH-141148: Ensure decimal.getcontext() returns fresh copy. #151953
Open
@nascheme

nascheme commented Jun 23, 2026

Copy link
Copy Markdown
Member Author

Switching this to a draft, since I feel like GH-151953 could be the better solution.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nascheme
nascheme marked this pull request as draft June 23, 2026 04:07
@bedevere-app bedevere-app Bot removed the awaiting core review label Jun 23, 2026
@github-actions github-actions Bot removed the stale Stale PR or inactive for long period of time. label Jun 23, 2026
@github-actions

github-actions Bot commented Sep 22, 2026

Copy link
Copy Markdown

This PR is stale because it has been open for 90 days with no activity.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@github-actions github-actions Bot added the stale Stale PR or inactive for long period of time. label Sep 22, 2026
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
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

Uh oh!

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

Assignees

No one assigned

Labels

stale Stale PR or inactive for long period of time.

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.

3 participants

@nascheme @skirpichev @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 01402503 — 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.