From 8a941de4cd79e86957e00a44d73768d24dbaebfd Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 14:34:53 -0400 Subject: [PATCH] fix(abcip): keep a sync Dispose, but one that never blocks on the teardown F10 follow-through: removing IDisposable outright broke a synchronous drv.Dispose() call site outside this lane (Runtime.Tests AbCipAlmaNativeAlarmEndToEndTests). The sync Dispose() is back, but it starts the same teardown as DisposeAsync and returns instead of parking the caller on DisposeAsync().AsTask().GetAwaiter().GetResult(); a background fault is logged. The runtime still takes the IAsyncDisposable path (preferred, never both). Refs #737 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../AbCipDriver.cs | 22 ++++++++++++-- .../AbCipLifecycleConcurrencyTests.cs | 29 ++++++++++++++++--- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 43afe57f..8e19daed 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -27,7 +27,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip; /// handles GC-collected tags. /// public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, IBatchSubscribable, - IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IAsyncDisposable + IHostConnectivityProbe, IPerCallHostResolver, IAlarmSource, ISkippedTagReporter, IAsyncDisposable, IDisposable { /// Authored tags this driver could not build into its poll table. private readonly SkippedTagLedger _skippedTags = new(); @@ -2268,8 +2268,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Asynchronously releases all resources used by the driver — the same complete teardown as /// . Idempotent, and safe CONCURRENTLY with a still-running /// (the runtime's final stop abandons a shutdown that overruns its budget and - /// disposes anyway): each teardown retires a disjoint device table. There is deliberately no synchronous - /// Dispose() — it could only block on this teardown (sync-over-async). + /// disposes anyway): each teardown retires a disjoint device table. This is the dispose the runtime calls + /// (it prefers ). /// /// A value task representing the asynchronous disposal. public async ValueTask DisposeAsync() @@ -2277,6 +2277,22 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, await ShutdownAsync(CancellationToken.None).ConfigureAwait(false); } + /// + /// Starts the same teardown as WITHOUT blocking the calling thread on it (review + /// 07 F10: the old DisposeAsync().AsTask().GetAwaiter().GetResult() parked the caller — a dispatcher + /// thread — for up to the probe join, with the usual sync-over-async deadlock exposure). The teardown runs to + /// completion in the background and any fault is logged. Callers that need completion use + /// ; kept only for synchronous using / Dispose() call sites. + /// + public void Dispose() + { + var teardown = DisposeAsync().AsTask(); + if (teardown.IsCompleted) { teardown.GetAwaiter().GetResult(); return; } + _ = teardown.ContinueWith( + t => _logger.LogWarning(t.Exception, "AbCip driver {DriverInstanceId}: background dispose failed", _driverInstanceId), + CancellationToken.None, TaskContinuationOptions.OnlyOnFaulted, TaskScheduler.Default); + } + /// /// Per-device runtime state. Holds the parsed host address, family profile, and the /// live libplctag.NET instances keyed by tag name. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs index eea409d2..2b009a7f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipLifecycleConcurrencyTests.cs @@ -175,12 +175,10 @@ public sealed class AbCipLifecycleConcurrencyTests created.Where(t => !t.Disposed).ShouldBeEmpty("every runtime a reader created must be disposed by some teardown"); } - /// F10: the driver exposes only an async dispose; a sync Dispose() that blocked on the async - /// teardown (sync-over-async) is gone, and the runtime takes the path. + /// F10: DisposeAsync is the complete, idempotent teardown (the runtime prefers it). [Fact] - public async Task Driver_is_async_disposable_only_and_DisposeAsync_stops_the_probe() + public async Task DisposeAsync_is_a_complete_idempotent_teardown() { - typeof(IDisposable).IsAssignableFrom(typeof(AbCipDriver)).ShouldBeFalse(); var (drv, factory) = Build(probeReadDelay: TimeSpan.Zero); await drv.InitializeAsync("{}", CancellationToken.None); await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); @@ -192,4 +190,27 @@ public sealed class AbCipLifecycleConcurrencyTests loop.IsCompleted.ShouldBeTrue(); factory.Tags["ProbeTag"].Disposed.ShouldBeTrue(); } + + /// + /// F10: the sync Dispose() no longer blocks the caller's thread on the async teardown (sync-over-async — + /// a dispatcher thread for up to the probe join). It starts the same teardown and returns; the teardown still + /// completes. + /// + [Fact] + public async Task Sync_Dispose_does_not_block_on_the_teardown_but_still_completes_it() + { + var (drv, factory) = Build(probeReadDelay: TimeSpan.FromMilliseconds(500)); + await drv.InitializeAsync("{}", CancellationToken.None); + await WaitUntilAsync(() => factory.Tags.ContainsKey("ProbeTag"), "probe never started"); + var loop = drv.GetDeviceState(Host)!.ProbeTask!; + await Task.Delay(20); // the probe is inside its slow, cancellation-deaf read + + var sw = System.Diagnostics.Stopwatch.StartNew(); + ((IDisposable)drv).Dispose(); + sw.Stop(); + + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromMilliseconds(250)); // did not wait out the 500 ms probe read + await WaitUntilAsync(() => loop.IsCompleted && factory.Tags["ProbeTag"].Disposed, "teardown never completed"); + drv.GetDeviceState(Host).ShouldBeNull(); + } }