fix: stop concurrent parses from sharing one serialization context (#327)

[livingstaccato]

Fixes #327. Fixes #344.

What

serialize() declared context=SerializationContext() as a default argument. Python evaluates that once, at import, so every rule in the process shared one mutable context -- and expressions.py, functions.py and indexing.py mutate it in place through context.modify(inside_dollar_string=True).

A thread serializing a function call therefore set inside_dollar_string for every other thread, and any tuple or object those threads were serializing came back as its inline HCL source ('[1, 2, 3]') instead of a list. Silently -- no exception, a different type. Downstream that is worse than a crash: code expecting a list gets a string and iterates its characters, so a tuple of resource names can become a sequence of single characters with nothing to indicate anything went wrong.

The four structural rules now take context=None, build a fresh SerializationContext when called without one, and thread the context they were given into their children, so one parse can no longer observe another's state.

That fixes the contexts the package builds for itself, and not the other half of the same defect: a consumer who builds a context and hands it to concurrent calls still had several writers on one object, because modify set fields on whatever context it was given. So SerializationContext is now frozen=True, modify is gone, and a traversal descends by building a child with replace. The checks that follow each of those seventeen scopes are unchanged in meaning -- they already read the outer value, which is now simply the context nothing wrote to.

Every other rule's serialize carried the same declaration and is changed with them, and so does options. Nothing in the package assigns to a SerializationOptions, so that one was never a live defect -- but it is the same construct in the same position, one mutable object handed to every caller that omits the argument and reachable by any subclass or hook a consumer writes, and keeping it meant defending a distinction that rests on nobody ever writing to it. Nothing reachable from loads used those defaults -- the public API always enters at StartRule -- so that half removes a trap rather than a live defect: a caller serializing a rule directly, outside the public API, still shared one context object with every other thread doing the same. The nine methods that never read the context keep the parameter and skip the construction.

Reproduction

Interleaving a document that toggles the flag with one that reads it, across 8 threads: 400 of 400 plain parses came back corrupted before the fix, 0 of 400 after. Below roughly 400 iterations the threads do not overlap enough to observe it at all -- at 100 iterations the unfixed code corrupts none -- which is why the regression test in test/unit/test_thread_safety.py runs 800.

Tests

The 800-parse reproduction is kept, because it is how the defect was found, but it can only ever be evidence: on a single-core or differently-scheduled worker its two halves may never meet. Two deterministic tests sit alongside it -- one holds a mutated context open on a barrier while another thread serializes, one asserts two serializations are handed different context objects at all -- and both fail against the unfixed rules without depending on the scheduler.

A third pins why options may keep the shared default that the context could not: nothing in the package assigns to it. If something ever does, that test fails rather than the default quietly becoming a second cross-thread channel.

For the immutability half, racing two threads proves nothing: modify set the field and restored it inside one call, so the window is far too small to land on by repetition -- a test that tried came back green against the unfixed code, and was thrown away rather than kept as false assurance. What discriminates is reading the caller's own context from inside the scope that used to mutate it: [True] before the change, [False] after.

Compatibility

The parameter keeps its name and stays optional, so the calling convention is preserved and a caller passing its own context still works exactly as before -- and now sharing one across concurrent parses is safe rather than merely permitted, which is what #344 asked for. Single-threaded behaviour is unchanged.

What freezing costs

Three things stop working, all of them on SerializationContext itself:

	before	after
with ctx.modify(x=True):	works	AttributeError -- use replace
ctx.inside_dollar_string = True	works	FrozenInstanceError
@dataclass class Sub(SerializationContext)	works	TypeError: cannot inherit non-frozen dataclass from a frozen one

The third is the one that fires furthest from its cause: at class definition rather than at use, so it surfaces on import with nothing pointing at the upgrade. The remedy is @dataclass(frozen=True) on the subclass. (A context also becomes hashable, which nothing needs but nothing minds.)

How much that costs depends on whether any of it was ever offered, and by this repository's own reckoning it was not. hcl2/__init__.py lists SerializationOptions in __all__ and does not list SerializationContext. The word "context" does not occur anywhere in docs/. Every documented serialization path is hcl2.serialize(tree, serialization_options=...), whose context parameter is not reachable at all -- the options are keyword-only and there is no context keyword. Reaching one means from hcl2.utils import SerializationContext, an unexported name from an undocumented module; StartRule is exported and takes a context positionally, but there is still no offered way to construct the argument. Inside the tree, modify had no callers outside hcl2/rules/ and its own tests.

So for anyone using the documented API this is invisible, and for anyone who went into hcl2.utils for a name the package declines to export, it is three loud failures rather than a silent change of behaviour.

The alternative, considered and not taken: thread replace through the seventeen call sites and leave the dataclass mutable. Nothing in hcl2/ assigns to a context, so that removes every actual mutation and breaks nothing -- modify could stay, unused. It also leaves the guarantee resting on nobody writing one back, which is the property that made #344 hard to see in the first place. Happy to take that version instead if you would rather not spend the break; it is a policy call rather than a technical one.

Merging

It touches the same code as #348 (hcl2/rules/expressions.py). Whichever of those lands first, this one needs a rebase rather than a merge — the overlaps are real edits to the same methods, not adjacent lines, so resolving them by hand risks losing one of the two fixes. Say the word and I will rebase and re-run the suite.

---

This pull request, 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]

Please hold off on merging this one for now — I want to do another review pass over it before it goes in. Opened as a draft for that reason; I will mark it ready and say so here once I am done.

[livingstaccato]

Review pass done, so the hold above no longer applies — this is ready for review now.

Rebased on current main; GitHub reports it mergeable as it stands.

_{🤖 Drafted with Claude Code.}