GH-141148: ensure tasks/threads get fresh copy of decimal.Context#146482
Draft
nascheme wants to merge 3 commits intopython:mainfrom
Draft
GH-141148: ensure tasks/threads get fresh copy of decimal.Context#146482nascheme wants to merge 3 commits intopython:mainfrom
nascheme wants to merge 3 commits intopython:mainfrom
Conversation
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.
Member
|
I think it's a correct approach. This introduce new C function, shouldn't it be discussed first with the C-API WG? |
Member
Author
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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ensure that
decimal.getcontext()returns a per-task copy of thedecimal.Contextso that mutations are isolated between asyncio tasks and threads whensys.flags.thread_inherit_contextis set.This change is required because
decimal.Contextinstances are mutable. Thecontextvars.Contextobject 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 thedecimal.Contextobject to ensure that potential mutations to it are isolated.There is a downside to this change. The
contextvars.Contextobject has gained an additional pointer valuePyHamtObject *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 thectx_vars_originobject is likely still alive anyhow, due to that task holding a reference to it. It is only if the original task callsContextVar.set()that you might free something.📚 Documentation preview 📚: https://cpython-previews--146482.org.readthedocs.build/