From 97a40f3ac2297ead7104a7af08106fd5697f8629 Mon Sep 17 00:00:00 2001
From: ZGaetano <zgaetano@wilddragon.net>
Date: Sat, 13 Jun 2026 00:22:55 -0400
Subject: [PATCH] fix(engine): don't leave a disposed finder behind on rebuild
 failure
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

RebuildFinder disposed the live finder *before* creating the replacement.
If CreateFinder threw (the documented failure mode the catch handles), the
service was left holding a disposed _finder, and every subsequent PollOnce
called GetCurrentSources on a dead handle — so the self-heal path could
permanently brick discovery, the exact outcome it exists to prevent. The
log even claimed "continuing with existing finder" while the finder was
gone.

Now we build the replacement into a local first and only dispose the old
finder once the new one is in hand. On failure the original finder stays
live and the seen-set is left intact (no spurious re-Add storm). Made the
method internal + bool-returning so the failure path is unit-testable.
---
 .../Discovery/NdiDiscoveryService.cs          | 33 +++++++++++++++----
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/src/Dragon-ISO.Engine/Discovery/NdiDiscoveryService.cs b/src/Dragon-ISO.Engine/Discovery/NdiDiscoveryService.cs
index 4a0e4a2..f8b7e58 100644
--- a/src/Dragon-ISO.Engine/Discovery/NdiDiscoveryService.cs
+++ b/src/Dragon-ISO.Engine/Discovery/NdiDiscoveryService.cs
@@ -187,23 +187,42 @@ public sealed class NdiDiscoveryService
     }
 
     /// <summary>
-    /// Dispose the current finder and create a fresh one against the cached
-    /// discovery groups. Clears the seen-set so all currently-visible sources
-    /// will re-fire as <see cref="DiscoveryEvent.Added"/> on the next poll.
+    /// Create a fresh finder against the cached discovery groups and, only once it
+    /// is successfully in hand, dispose the old one and clear the seen-set so all
+    /// currently-visible sources re-fire as <see cref="DiscoveryEvent.Added"/> on
+    /// the next poll.
+    ///
+    /// Ordering matters: we build the replacement <em>before</em> disposing the
+    /// incumbent. If <see cref="INdiInterop.CreateFinder"/> throws (the failure mode
+    /// this method's catch exists for), the incumbent finder is left untouched and
+    /// fully usable, the seen-set is preserved (no spurious re-Add storm), and the
+    /// loop simply keeps polling the existing finder until the next rebuild attempt.
+    ///
+    /// Returns true if the finder was actually swapped; false if the rebuild failed
+    /// and the existing finder was retained. Internal for unit-testing the failure
+    /// path.
     /// </summary>
-    private void RebuildFinder(string reason)
+    internal bool RebuildFinder(string reason)
     {
+        NdiFindHandle replacement;
         try
         {
             _logger.LogInformation("Rebuilding NDI finder ({Reason}).", reason);
-            _finder.Dispose();
-            _finder = _interop.CreateFinder(_discoveryGroups);
-            _previous.Clear();
+            replacement = _interop.CreateFinder(_discoveryGroups);
         }
         catch (Exception ex)
         {
             _logger.LogWarning(ex, "Finder rebuild failed ({Reason}); continuing with existing finder.", reason);
+            return false;
         }
+
+        // New finder is live — retire the old one and reset discovery state.
+        var old = _finder;
+        _finder = replacement;
+        try { old.Dispose(); }
+        catch (Exception ex) { _logger.LogWarning(ex, "Disposing previous finder threw after rebuild ({Reason}).", reason); }
+        _previous.Clear();
+        return true;
     }
 
     /// <summary>