fix(justdummies): make UseSeed scopes safe under out-of-order disposal - #323
Merged
Conversation
A seed scope kept only a snapshot of the ambient state at open time and restored it blindly on Dispose, which is correct only for LIFO disposal. Any other order either unpinned a still-open inner scope or resurrected a closed scope's seed as the ambient context for whatever ran next. Turn the ambient state into a linked stack of frames: Dispose tombstones its own frame, and only the current top rewrites the AsyncLocal slot, walking past tombstoned ancestors to the nearest live frame. LIFO stays bit-for-bit identical; out-of-order disposal no longer strands or leaks; double-dispose remains a no-op. This makes the code honour the contract UseSeed already documents. Refs: #321
Reconcile the out-of-order-disposal fix with ADR-0045 (guard public and internal arguments against null), which landed on main via #322. * AmbientState constructor: main added a null guard on `random`; this branch added a `parent` parameter to the same constructor — keep both. * SeedScope: bring its new internal constructor under the same convention by guarding its non-nullable `frame` argument, matching the guard #322 applied to AmbientState. The reflection-driven NullArgumentGuardConventionTests added by #322 passes against the merged code.
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
2 participants
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.
Summary
Any.UseSeed(...)returns a disposable seed scope that is documented to nest — "disposing restores whatever was pinned before, and disposing twice is harmless." The implementation only honoured that for LIFO disposal: it kept a singleAsyncLocalslot and each scope blindly restored its snapshot-at-open onDispose. Any other disposal order either unpinned a still-open inner scope or resurrected a closed scope's seed as the ambient context for whatever ran next. This makes disposal order-independent, honouring the contractUseSeedalready documents.Type of change
Changes
JustDummies/RandomSource.cs— turn the ambient seed state into a linked stack of frames. Each frame remembers its parent;SeedScope.Disposetombstones its own frame, and only the current top rewrites theAsyncLocalslot, walking past tombstoned ancestors to the nearest still-open frame (ornull). LIFO disposal stays bit-for-bit identical; out-of-order disposal no longer strands a still-open inner scope (repro 2 in JustDummies: Any.UseSeed scopes are not safe under out-of-order disposal #321) or leaks a closed scope's seed to what runs next (repro 1); double-dispose remains a no-op.JustDummies.UnitTests/AmbientSeedScopeTests.cs— three named regression tests (specific structural cases, ADR-0040), each confirmed red against the old blind-restore before the fix:Testing
dotnet build FirstClassErrors.sln— 0 errors (one transient file-lock copy-retry warning from a concurrent process, no code warning).dotnet test FirstClassErrors.sln— not run in full locally; ran the affected suites instead:JustDummies.UnitTests(558),JustDummies.PropertyTests(220) andJustDummies.Xunit.UnitTests(14, the adapter that shares this scope machinery) — all green. The three new tests were confirmed red before the fix.build/stryker/justdummies.json --since) launched locally on the diff; CI runs it authoritatively on this PR.Documentation
Any.UseSeed's XML docs already promise; no user-facing behaviour changes for correct (LIFO) callers.Architecture decisions
Related issues
Closes #321
Generated by Claude Code