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 1ee8a308..17a83f42 100644 --- a/JustDummies/RandomSource.cs +++ b/JustDummies/RandomSource.cs @@ -133,10 +133,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 @@ -147,7 +147,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; } @@ -175,35 +175,62 @@ 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) { if (random is null) { throw new ArgumentNullException(nameof(random)); } 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) { + if (frame is null) { throw new ArgumentNullException(nameof(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; + } } }