diff --git a/Motely.Tests/MotelySearchReliabilityTests.cs b/Motely.Tests/MotelySearchReliabilityTests.cs new file mode 100644 index 00000000..92e384a0 --- /dev/null +++ b/Motely.Tests/MotelySearchReliabilityTests.cs @@ -0,0 +1,121 @@ +using Motely.Filters; +using Xunit; + +namespace Motely.Tests; + +/// +/// Reliability regressions for . +/// Both tests covered HIGH-severity items in ISSUES.md: +/// 1. Single-thread used to run the +/// worker body synchronously on the caller, so awaiting it on the same context deadlocked. +/// 2. Multi-thread workers used to swallow exceptions silently — the completion source +/// stayed unset and callers hung forever. +/// +public sealed class MotelySearchReliabilityTests +{ + /// Always-pass base filter so the worker actually runs for the seed list. + private readonly struct PassFilterDesc : IMotelySeedFilterDesc + { + public PassFilter CreateFilter(ref MotelyFilterCreationContext ctx) => new(); + + public readonly struct PassFilter : IMotelySeedFilter + { + public readonly VectorMask Filter(ref MotelyVectorSearchContext _) => + VectorMask.AllBitsSet; + } + } + + /// Base filter that throws inside the SIMD loop. + private readonly struct ThrowingFilterDesc : IMotelySeedFilterDesc + { + public ThrowingFilter CreateFilter(ref MotelyFilterCreationContext ctx) => new(); + + public readonly struct ThrowingFilter : IMotelySeedFilter + { + public readonly VectorMask Filter(ref MotelyVectorSearchContext _) => + throw new InvalidOperationException("boom from worker"); + } + } + + [Fact] + public async Task RunSearchAsync_SingleThread_DoesNotDeadlockAwaitOnSameContext() + { + var settings = new MotelySearchSettings(new PassFilterDesc()) + .WithListSearch(["AAAAAAAA"], 1) + .WithThreadCount(1) + .WithQuietMode(true); + + using var search = settings.CreateSearch(); + + // The bug under fix: when totalWorkers == 1 the worker body used to run + // synchronously inside Start(), meaning RunSearchAsync()'s returned Task + // only completed after the search had already drained on the caller's + // thread — fatal for `await search.RunSearchAsync()` on a sync ctx. + // A bounded wait proves the Task yields and completes off-thread. + var run = search.RunSearchAsync(); + var winner = await Task.WhenAny(run, Task.Delay(TimeSpan.FromSeconds(5))); + Assert.Same(run, winner); + await run; // no exception → completed cleanly + Assert.True(search.IsCompleted); + } + + [Fact] + public async Task RunSearchAsync_SingleThread_SurfacesWorkerException() + { + var settings = new MotelySearchSettings(new ThrowingFilterDesc()) + .WithListSearch(["AAAAAAAA"], 1) + .WithThreadCount(1) + .WithQuietMode(true); + + using var search = settings.CreateSearch(); + var ex = await Assert.ThrowsAnyAsync(() => search.RunSearchAsync()); + Assert.Contains("boom from worker", FlattenMessages(ex)); + } + + [Fact] + public async Task RunSearchAsync_MultiThread_SurfacesWorkerException() + { + // Many lanes / multiple workers so the throw lands on a worker thread, + // not the caller. Previously this would set _completionSource to nothing + // and the await would hang forever. + var seeds = Enumerable.Range(0, 1024).Select(i => $"S{i:D7}").ToArray(); + var settings = new MotelySearchSettings(new ThrowingFilterDesc()) + .WithListSearch(seeds, seeds.Length) + .WithThreadCount(Math.Max(2, Environment.ProcessorCount)) + .WithQuietMode(true); + + using var search = settings.CreateSearch(); + var run = search.RunSearchAsync(); + var winner = await Task.WhenAny(run, Task.Delay(TimeSpan.FromSeconds(5))); + Assert.Same(run, winner); // didn't hang + var ex = await Assert.ThrowsAnyAsync(() => run); + Assert.Contains("boom from worker", FlattenMessages(ex)); + } + + [Fact] + public void RunSearchUntilCompletion_MultiThread_SurfacesWorkerException() + { + var seeds = Enumerable.Range(0, 1024).Select(i => $"S{i:D7}").ToArray(); + var settings = new MotelySearchSettings(new ThrowingFilterDesc()) + .WithListSearch(seeds, seeds.Length) + .WithThreadCount(Math.Max(2, Environment.ProcessorCount)) + .WithQuietMode(true); + + using var search = settings.CreateSearch(); + // Sync surface: previously the throw was lost and SignalSearchCompleted() + // marked the search "clean". Now the first error rethrows. + var ex = Assert.ThrowsAny(() => search.RunSearchUntilCompletion()); + Assert.Contains("boom from worker", FlattenMessages(ex)); + } + + private static string FlattenMessages(Exception ex) + { + var msgs = new List(); + for (var current = ex; current is not null; current = current.InnerException) + msgs.Add(current.Message); + if (ex is AggregateException agg) + foreach (var inner in agg.Flatten().InnerExceptions) + msgs.Add(inner.Message); + return string.Join(" | ", msgs); + } +} diff --git a/Motely/MotelySearch.cs b/Motely/MotelySearch.cs index 9f2f942d..0e7a6a27 100644 --- a/Motely/MotelySearch.cs +++ b/Motely/MotelySearch.cs @@ -1095,10 +1095,28 @@ public void RunSearchUntilCompletion() throw new InvalidOperationException("Search has already been started."); _elapsedTime.Start(); + Exception? firstError = null; + + void RunWorkerSafe(int idx) + { + try + { + RunWorkerBody(_plans[idx]); + } + catch (OperationCanceledException) when (_cancellationToken.IsCancellationRequested) + { + // cooperative cancellation + } + catch (Exception ex) + { + Interlocked.CompareExchange(ref firstError, ex, null); + } + } + if (_threadCount == 1) { // Single-threaded: run directly on this thread - RunWorkerBody(_plans[0]); + RunWorkerSafe(0); } else { @@ -1107,7 +1125,7 @@ public void RunSearchUntilCompletion() for (int i = 0; i < _threadCount; i++) { int threadIdx = i; - threads[i] = new Thread(() => RunWorkerBody(_plans[threadIdx])) + threads[i] = new Thread(() => RunWorkerSafe(threadIdx)) { Name = $"Motely Search Thread {threadIdx}", IsBackground = true @@ -1121,6 +1139,15 @@ public void RunSearchUntilCompletion() } } + if (firstError is not null) + { + // Tell awaiters about the failure, then rethrow on the caller for the + // sync surface. Without this, a failed worker used to look like a clean + // completion to anyone waiting on _completionSource. + _completionSource.TrySetException(firstError); + throw firstError; + } + SignalSearchCompleted(); } @@ -1195,48 +1222,85 @@ private void BeginSearch(CancellationToken cancellationToken) /// Starts search threads without blocking the caller. /// Completion is signaled via . /// + /// + /// Both paths off-thread the worker bodies. The single-thread case used to run + /// synchronously on the caller, which broke : the Task + /// only returned after the search had already completed, so any code that did + /// await search.RunSearchAsync() on the same thread deadlocked. The + /// multi-thread case used to swallow worker exceptions silently — if a worker + /// threw, never moved and the caller hung forever. + /// Every worker is now wrapped so the first exception is captured and surfaced + /// through once the last worker drops out. + /// private void StartSearchThreads() { - // what the fuck - pifreak - // //ObjectDisposedException.ThrowIf(Volatile.Read(ref _isDisposed) != 0, this); - _elapsedTime.Start(); - if (_threadCount == 1) + int totalWorkers = _threadCount; + WorkerCoordinator coordinator = new(this, totalWorkers); + + for (int i = 0; i < totalWorkers; i++) { + int threadIdx = i; + var thread = new Thread(() => coordinator.RunWorker(threadIdx)) + { + Name = totalWorkers == 1 + ? "Motely Search Thread" + : $"Motely Search Thread {threadIdx}", + IsBackground = true, + }; + thread.Start(); + } + } - try - { - RunWorkerBody(_plans[0]); - SignalSearchCompleted(); - } - catch (Exception ex) - { - _completionSource.TrySetException(ex); - } + /// + /// Tracks the running worker count and routes the first thrown exception back + /// through . One instance per search. + /// + private sealed class WorkerCoordinator + { + private readonly MotelySearch _owner; + private int _remaining; + private Exception? _firstError; + + public WorkerCoordinator(MotelySearch owner, int totalWorkers) + { + _owner = owner; + _remaining = totalWorkers; } - else + + public void RunWorker(int idx) { - int remaining = _threadCount; - for (int i = 0; i < _threadCount; i++) + try { - int threadIdx = i; - var thread = new Thread(() => + _owner.RunWorkerBody(_owner._plans[idx]); + } + catch (OperationCanceledException) when (_owner._cancellationToken.IsCancellationRequested) + { + // honour cooperative cancellation — no error + } + catch (Exception ex) + { + Interlocked.CompareExchange(ref _firstError, ex, null); + } + finally + { + if (Interlocked.Decrement(ref _remaining) == 0) { - RunWorkerBody(_plans[threadIdx]); - if (Interlocked.Decrement(ref remaining) == 0) + Thread.MemoryBarrier(); + var err = Volatile.Read(ref _firstError); + if (err is not null) + { + _owner._completionSource.TrySetException(err); + } + else { - Thread.MemoryBarrier(); bool completed = - Volatile.Read(ref _isDisposed) == 0 && !_cancellationToken.IsCancellationRequested; - _completionSource.TrySetResult(completed); + Volatile.Read(ref _owner._isDisposed) == 0 + && !_owner._cancellationToken.IsCancellationRequested; + _owner._completionSource.TrySetResult(completed); } - }) - { - Name = $"Motely Search Thread {threadIdx}", - IsBackground = true - }; - thread.Start(); + } } } }