fix(engine): don't leave a disposed finder behind on rebuild failure
All checks were successful
CI / build-and-test (push) Successful in 27s
All checks were successful
CI / build-and-test (push) Successful in 27s
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.
This commit is contained in:
parent
e5f10f2667
commit
97a40f3ac2
1 changed files with 26 additions and 7 deletions
|
|
@ -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>
|
||||
|
|
|
|||
Loading…
Reference in a new issue