`SerializationContext.modify` mutates in place, so a caller-supplied context cannot be shared across threads

[livingstaccato]

Summary

SerializationContext.modify is a context manager that mutates the context in
place and restores the previous values on exit (hcl2/utils.py:134 at
v8.1.3). Every rule that needs to change the context does so through it, on
whatever object it was handed.

That makes a caller-supplied context unsafe to share. serialize() accepts one,
threads it through the whole tree, and the rules underneath write to it, so two
concurrent parses sharing a single context observe each other's flags exactly as
two parses sharing the module-level default used to.

This is not the defect in #327. That one is about the implicit shared object
created by context=SerializationContext() as a default argument, and removing
it does nothing for a context the caller creates and shares deliberately.

Reproduction

from concurrent.futures import ThreadPoolExecutor

from hcl2.api import parses
from hcl2.utils import SerializationContext, SerializationOptions

TOGGLES = "z = f([1, 2, 3], {a = 1})\n"
PLAIN = "x = [1, 2, 3]\ny = {a = 1}\n"
EXPECTED = {"x": [1, 2, 3], "y": {"a": 1}}

shared = SerializationContext()
options = SerializationOptions()


def work(index):
    tree = parses(TOGGLES if index % 2 else PLAIN)
    result = tree.serialize(options, shared)
    return None if index % 2 else result


with ThreadPoolExecutor(max_workers=8) as pool:
    results = [r for r in pool.map(work, range(6000)) if r is not None]
print(sum(r != EXPECTED for r in results), "of", len(results), "corrupted")

Three consecutive runs on the branch that fixes #327: 1635, 2228 and 2591 of
3000 parses corrupted. A tuple comes back as its inline HCL source
('[1, 2, 3]') rather than a list, with no exception raised. On released 8.1.3
the same script reports 31 of 400 at the smaller size the shared default already
made observable.

Suggested shape of the fix

Make the context immutable and have modify return a new one rather than
writing to the caller's. The class already carries exactly that method, and it
has no callers anywhere in the package:

def replace(self, **kwargs) -> "SerializationContext":
    """Return a new context with the given fields overridden."""
    return replace(self, **kwargs)

Every with context.modify(...) block would become a rebind that is passed to
the children it wraps, which the rules already do for the context itself. With
@dataclass(frozen=True) on top, a shared context stops being a hazard by
construction rather than by convention, and the fix for #327 becomes an
optimisation rather than a correctness fix.

It is a bigger change than #327 -- every mutation site moves -- which is why
this is filed separately rather than folded into that PR.

---

This issue, and the investigation behind it, were produced by an AI assistant (Claude) working on behalf of the author. Please review with that provenance in mind.

[livingstaccato]

A suggestion on how this might be taken, since the shape of the fix is a judgement call rather than a correction.

The narrow version is mechanical and self-contained: make SerializationContext a frozen dataclass, and have modify return a new context instead of writing to the caller's. The class already carries replace(), which does exactly that and has no callers, so the pieces are in place. Every with context.modify(...) block becomes a rebind passed to the children it wraps — which the rules already do with the context itself, so the shape of the call sites does not change, only what they hand down.

That has a property worth stating plainly: it makes a shared context safe by construction rather than by convention. #327's fix removes the shared object nobody asked for; this removes the hazard from the one a caller creates deliberately. With it in place, #327 becomes an optimisation rather than a correctness fix.

The cost is that it touches every mutation site in one change, and modify is public. If a consumer has written a rule that calls it, their rule keeps working — the context manager still exists — but a rule that mutated the context expecting the change to outlive the block would stop working. That is a real if unlikely break, and probably wants a minor version rather than a patch.

Happy to take this if it is wanted; leaving it filed rather than guessing at the version policy.

[livingstaccato]

Update: this is implemented in #334, which closes it alongside #327 — SerializationContext is a frozen dataclass, replace() returns a new context, every with context.modify(...) site is converted to a rebind, and modify is removed.

The version-policy note above needs correcting rather than restating: modify is removed outright, not preserved, so a consumer calling it gets an AttributeError rather than the subtler change I described there. That is a larger break than this issue states, and firmly a minor rather than a patch.

The offer above is out of date — the work is written and up for review.

_{🤖 Drafted with Claude Code.}