With the addition of the thread_inherit_context system flag, we now have an issue with decimal.getcontext(). It is supposed to return a separate decimal.Context instance for each thread. Before the thread_inherit_context change, this worked because each new thread started with an empty contextvars.Context instance. The code for getcontext() was:
def getcontext():
"""Returns this thread's context.
If this thread does not yet have a context, returns
a new context and sets this thread's context.
New contexts are copies of DefaultContext.
"""
try:
return _current_context_var.get()
except LookupError:
context = Context()
_current_context_var.set(context)
return context
So a new thread would hit the LookupError case and create its own instance. This ensured that each thread had its own decimal.Context instance and that mutations to it were isolated. Now, when the thread_inherit_context flag is set, threads inherit the contextvars.Context from the thread spawning them. So the LookupError case is not hit and the decimal.Context instance is shared between the new thread and the spawning thread. Since the decimal.Context instance is mutable (e.g. getcontext().prec = NN is a common pattern), this leads to trouble.
Note that getcontext() is arguably broken when used by asyncio tasks, even before the addition of the thread_inherit_context. A new task inherits contextvars.Context (it calls copy() and then uses Context.run() to run the task with the copied contextvar bindings), so the same sharing problem exists.
A possible fix is to make it so that getcontext() can detect when the decimal.Context instance needs to be copied. This can be done fairly efficiently with the addition of a ContextVar.get_changed() method. Then the code for getcontext() can be:
def getcontext():
"""Returns this thread's context.
If this thread does not yet have a context, returns
a new context and sets this thread's context.
New contexts are copies of DefaultContext.
"""
try:
context, changed = _current_context_var.get_changed()
except LookupError:
context = Context()
_current_context_var.set(context)
return context
if not changed:
# The context value was inherited from another task/thread. Because
# the Context() instance is mutable, copy it to ensure that if it is
# changed, those changes are isolated from other tasks/threads.
context = context.copy()
_current_context_var.set(context)
return context
My PR also adds a new C-API: PyContextVar_GetChanged. This is used by the C implementation of the decimal module.
Performance: the implementation snapshots the HAMT root pointer at Context.run() enter time (one extra Py_NewRef/Py_CLEAR per enter/exit). Since HAMTs are persistent, this just keeps the old root alive, the new tree shares all unchanged nodes, so the extra memory is a few superseded internal nodes at most.
I haven’t been able to think of an alternative approach that doesn’t require something like get_changed(). Storing the current thread ID on the decimal.Context instance and
copying when it doesn’t match seems inelegant and doesn’t work for asyncio tasks.
Questions
- Does get_changed() as public API overly constrain future optimizations of the context implementation? With the current HAMT structure it’s cheap — just compare a key’s value in
the current vs. origin snapshot. The semantics only require knowing whether a var was set in the current scope, not full HAMT diffing. - Should this be a public API or a private one? It might be useful for copy-on-write patterns with mutable context state more generally, but it may also be a niche need that
only decimal has. If the consensus is that this is too specialized, it could be made private. - Is there an alternative approach I’m not seeing?