From b731c6cb5d8fea55de290651c422ea1902a903e5 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 27 Jul 2026 16:23:49 +0000 Subject: [PATCH] fix(justdummies): make UseSeed scopes safe under out-of-order disposal 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 --- .../AmbientSeedScopeTests.cs | 62 +++++++++++++++++++ JustDummies/RandomSource.cs | 53 +++++++++++----- 2 files changed, 101 insertions(+), 14 deletions(-) diff --git a/JustDummies.UnitTests/AmbientSeedScopeTests.cs b/JustDummies.UnitTests/AmbientSeedScopeTests.cs index da8b45a0..b06ad576 100644 --- a/JustDummies.UnitTests/AmbientSeedScopeTests.cs +++ b/JustDummies.UnitTests/AmbientSeedScopeTests.cs @@ -106,6 +106,68 @@ public void DisposingTwiceIsHarmless() { Check.ThatCode(() => scope.Dispose()).DoesNotThrow(); } + [Fact(DisplayName = "Disposing an outer scope out of order leaves the still-open inner scope's seed pinned.")] + public void OutOfOrderDisposalKeepsTheInnerScopePinned() { + // Reference: what seed 2 yields as the sole, top-of-stack scope. + (int, string) reference; + using (Any.UseSeed(2)) { reference = Batch(); } + + // Open two scopes, then dispose the OUTER first — the inner (seed 2) is still open, so the ambient + // context must still be pinned to seed 2. A blind restore-the-previous unpins it instead, leaving the + // still-open inner scope drawing from a fresh unseeded generator. + IDisposable outer = Any.UseSeed(1); + IDisposable inner = Any.UseSeed(2); + outer.Dispose(); + (int, string) whileInnerStillOpen = Batch(); + inner.Dispose(); + + Check.That(whileInnerStillOpen).IsEqualTo(reference); + } + + [Fact(DisplayName = "Out-of-order disposal does not leak a pinned seed to what runs next.")] + public void OutOfOrderDisposalDoesNotLeakASeed() { + // Reference: seed 1's sequence, to prove it is NOT what a later, unseeded draw replays. + (int, string) seedOneSequence; + using (Any.UseSeed(1)) { seedOneSequence = Batch(); } + + // Dispose the outer first, then the inner: a blind restore-the-previous now reinstates seed 1's frame, + // stranding it as the ambient context for whatever runs next. + IDisposable outer = Any.UseSeed(1); + IDisposable inner = Any.UseSeed(2); + outer.Dispose(); + inner.Dispose(); + + // Both scopes are closed, so the ambient context must be unseeded again — not pinned to seed 1. + (int, string) afterAllDisposed = Batch(); + + Check.That(afterAllDisposed).IsNotEqualTo(seedOneSequence); + } + + [Fact(DisplayName = "Disposing a middle scope out of order leaves the top scope and its live ancestors intact.")] + public void OutOfOrderMiddleDisposalPreservesTheStack() { + (int, string) seedThree; + using (Any.UseSeed(3)) { seedThree = Batch(); } + (int, string) seedOne; + using (Any.UseSeed(1)) { seedOne = Batch(); } + + IDisposable bottom = Any.UseSeed(1); + IDisposable middle = Any.UseSeed(2); + IDisposable top = Any.UseSeed(3); + + // Dispose the middle scope early: the top (seed 3) is untouched and stays pinned. + middle.Dispose(); + (int, string) topStillPinned = Batch(); + + // Now the top goes: it must skip the already-disposed middle and land on the bottom (seed 1). + top.Dispose(); + (int, string) afterTopDisposed = Batch(); + + bottom.Dispose(); + + Check.That(topStillPinned).IsEqualTo(seedThree); + Check.That(afterTopDisposed).IsEqualTo(seedOne); + } + [Fact(DisplayName = "The scope does not leak across parallel execution contexts.")] public async Task TheScopeDoesNotLeakAcrossExecutionContexts() { (int, string) inside; diff --git a/JustDummies/RandomSource.cs b/JustDummies/RandomSource.cs index bcfae48c..7adf9ff5 100644 --- a/JustDummies/RandomSource.cs +++ b/JustDummies/RandomSource.cs @@ -131,10 +131,10 @@ internal static IDisposable UseSeed(int seed) { } internal static IDisposable UseSeed(int seed, string? replaySnippet) { - AmbientState? previous = State.Value; - State.Value = new AmbientState(new SeededRandom(seed), replaySnippet); + AmbientState frame = new(new SeededRandom(seed), replaySnippet, State.Value); + State.Value = frame; - return new SeedScope(previous); + return new SeedScope(frame); } #endregion @@ -145,7 +145,7 @@ internal override SeededRandom Current { get { AmbientState? current = State.Value; if (current is null) { - current = new AmbientState(new SeededRandom(NewSeed()), null); + current = new AmbientState(new SeededRandom(NewSeed()), null, null); State.Value = current; } @@ -173,33 +173,58 @@ private static string ReplaySnippet(int seed) { #region Nested types - /// The ambient state a seed scope installs: the seeded generator, and how to replay the run that uses it. + /// + /// One frame of the ambient seed stack a scope installs: the seeded generator, how to replay the run that uses + /// it, and the frame it was pushed on top of. The frames form a linked stack (each points at its + /// ) so a scope disposed out of order can be removed without stranding the ones still open + /// — see . tombstones a frame whose scope has closed but which is + /// not yet the top of the stack, so the top's later disposal can skip past it. + /// private sealed class AmbientState { - internal AmbientState(SeededRandom random, string? replaySnippet) { + internal AmbientState(SeededRandom random, string? replaySnippet, AmbientState? parent) { Random = random; ReplaySnippet = replaySnippet; + Parent = parent; } - internal SeededRandom Random { get; } - internal string? ReplaySnippet { get; } + internal SeededRandom Random { get; } + internal string? ReplaySnippet { get; } + internal AmbientState? Parent { get; } + internal bool Disposed { get; set; } } + /// + /// The handle returned by . Disposal is order-independent: it + /// tombstones its own frame, and only the frame that is currently the top of the stack rewrites the ambient + /// slot — walking past any tombstoned ancestors to the nearest frame whose scope is still open (or to + /// null when none is). So the documented "scopes nest, disposing restores whatever was pinned before" + /// holds even when scopes are disposed out of order: an outer scope closed early strands nothing, and no order + /// leaves a dead seed pinned for whatever runs next. Disposing twice is a no-op. + /// private sealed class SeedScope : IDisposable { - private readonly AmbientState? _previous; - private bool _disposed; + private readonly AmbientState _frame; + private bool _disposed; - internal SeedScope(AmbientState? previous) { - _previous = previous; + internal SeedScope(AmbientState frame) { + _frame = frame; } public void Dispose() { if (_disposed) { return; } - _disposed = true; - State.Value = _previous; + _disposed = true; + _frame.Disposed = true; + + // Only the current top owns the ambient slot; an out-of-order dispose of an inner frame just tombstones + // it and lets the top's own dispose skip it later. + if (ReferenceEquals(State.Value, _frame)) { + AmbientState? restored = _frame.Parent; + while (restored is { Disposed: true }) { restored = restored.Parent; } + State.Value = restored; + } } }