diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index 5c4953d3..f65bb2f4 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -1779,6 +1779,13 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, return null; } + /// + /// How long a handle recycle waits for in-flight ops on the detached client: one call timeout (each op is + /// bounded by it) plus slack, so a recycle never kills a read that would have completed. + /// + private TimeSpan RecycleDrainBudget => + (_options.Timeout > TimeSpan.Zero ? _options.Timeout : TimeSpan.FromSeconds(2)) + TimeSpan.FromMilliseconds(500); + private async Task RecycleLoopAsync(DeviceState state, CancellationToken ct) { while (!ct.IsCancellationRequested) @@ -1788,14 +1795,31 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // Close the current handle — the next Read / Write / Probe call triggers // EnsureConnectedAsync, which reopens a fresh one. We don't block here on - // reconnect because the goal is just to release the FWLIB handle slot; a - // readable tick one probe cycle later is an acceptable cost. - try { state.DisposeClient(); } + // reconnect because the goal is just to release the FWLIB handle slot. + // + // Under the device's ConnectGate, exactly like EnsureConnectedAsync's install: unserialised, the + // recycle could null a client a concurrent connect had just installed (orphaning a live two-socket + // session) or dispose one a poll was mid-read on (BadCommunicationError + Degraded every interval). + // The gate is held through the drain so no fresh session opens beside the old one — the CNC's + // handle pool is small — and the detached client is disposed only once the ops already holding it + // have finished (bounded by the call timeout). + try { await state.ConnectGate.WaitAsync(ct).ConfigureAwait(false); } + catch (OperationCanceledException) { break; } + try + { + var old = state.Client; + state.Client = null; + if (old is SynchronizedFocasClient sync) + await sync.DisposeWhenIdleAsync(RecycleDrainBudget).ConfigureAwait(false); + else + old?.Dispose(); + } catch (Exception ex) { /* already disposed or race — next EnsureConnected recovers */ _logger.LogDebug(ex, "FOCAS handle-recycle dispose failed for {Host}", state.Options.HostAddress); } + finally { state.ConnectGate.Release(); } } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs index 9dc81fca..a0bade40 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/SynchronizedFocasClient.cs @@ -32,6 +32,9 @@ public sealed class SynchronizedFocasClient : IFocasClient private readonly IFocasClient _inner; private readonly TimeSpan _callTimeout; private readonly SemaphoreSlim _gate = new(1, 1); + /// Ops holding or queued for — what drains. + private int _pending; + private int _disposed; /// Wraps with per-device serialization + a per-call timeout. /// The underlying FOCAS client to serialize access to. @@ -115,26 +118,66 @@ public sealed class SynchronizedFocasClient : IFocasClient public Task> GetSpindleMotorSpeedsAsync(CancellationToken cancellationToken) => RunBoundedAsync(ct => _inner.GetSpindleMotorSpeedsAsync(ct), cancellationToken); - /// Disposes the inner client and the call gate. + /// + /// Disposes the inner client. Ops still queued on the call gate then fail with + /// instead of running on a disposed socket. The gate itself is NOT + /// disposed: an op that already holds it releases it on the way out, and releasing a disposed + /// threw out of a read that had otherwise completed. + /// public void Dispose() { + if (Interlocked.Exchange(ref _disposed, 1) != 0) return; _inner.Dispose(); - _gate.Dispose(); + } + + /// + /// Disposes once every op that already holds (or is queued for) this client has finished, waiting at most + /// . Used by the handle-recycle loop, which detaches the client under the device's + /// connect gate first — so nothing new can reach it — and then must not kill a read that is mid-flight on + /// it (that read used to surface BadCommunicationError + Degraded on every recycle tick). + /// + /// Upper bound on the wait; the client is disposed regardless once it passes. + /// A task that completes when the client is disposed. + public async Task DisposeWhenIdleAsync(TimeSpan maxWait) + { + var deadline = DateTime.UtcNow + maxWait; + while (Volatile.Read(ref _pending) > 0 && DateTime.UtcNow < deadline) + await Task.Delay(5, CancellationToken.None).ConfigureAwait(false); + Dispose(); + } + + /// Gets the number of ops currently holding or queued for the call gate. Test seam. + internal int PendingOperations => Volatile.Read(ref _pending); + + private void ThrowIfDisposed() + { + if (Volatile.Read(ref _disposed) != 0) + throw new ObjectDisposedException(nameof(SynchronizedFocasClient), "The FOCAS client was recycled or disposed."); } // Gate only — the caller already governs the budget (connect timeout arg / probe linked token). private async Task RunGatedAsync(Func> op, CancellationToken ct) { - await _gate.WaitAsync(ct).ConfigureAwait(false); - try { return await op(ct).ConfigureAwait(false); } - finally { _gate.Release(); } + Interlocked.Increment(ref _pending); + try + { + await _gate.WaitAsync(ct).ConfigureAwait(false); + try { ThrowIfDisposed(); return await op(ct).ConfigureAwait(false); } + finally { _gate.Release(); } + } + finally { Interlocked.Decrement(ref _pending); } } private async Task RunGatedAsync(Func op, CancellationToken ct) { - await _gate.WaitAsync(ct).ConfigureAwait(false); - try { await op(ct).ConfigureAwait(false); } - finally { _gate.Release(); } + Interlocked.Increment(ref _pending); + try + { + await _gate.WaitAsync(ct).ConfigureAwait(false); + try { ThrowIfDisposed(); await op(ct).ConfigureAwait(false); } + finally { _gate.Release(); } + } + finally { Interlocked.Decrement(ref _pending); } } // Gate + per-call timeout. A fired timeout surfaces as OperationCanceledException whose token is @@ -142,16 +185,22 @@ public sealed class SynchronizedFocasClient : IFocasClient // their own token's IsCancellationRequested. private async Task RunBoundedAsync(Func> op, CancellationToken ct) { - await _gate.WaitAsync(ct).ConfigureAwait(false); + Interlocked.Increment(ref _pending); try { - if (_callTimeout <= TimeSpan.Zero) - return await op(ct).ConfigureAwait(false); + await _gate.WaitAsync(ct).ConfigureAwait(false); + try + { + ThrowIfDisposed(); + if (_callTimeout <= TimeSpan.Zero) + return await op(ct).ConfigureAwait(false); - using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct); - linked.CancelAfter(_callTimeout); - return await op(linked.Token).ConfigureAwait(false); + using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct); + linked.CancelAfter(_callTimeout); + return await op(linked.Token).ConfigureAwait(false); + } + finally { _gate.Release(); } } - finally { _gate.Release(); } + finally { Interlocked.Decrement(ref _pending); } } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasHandleRecycleTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasHandleRecycleTests.cs index 97042231..4c287b4f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasHandleRecycleTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasHandleRecycleTests.cs @@ -62,6 +62,73 @@ public sealed class FocasHandleRecycleTests await drv.ShutdownAsync(CancellationToken.None); } + /// A client whose reads take a while — long enough for a recycle tick to land mid-read. + private sealed class SlowReadClient : FakeFocasClient + { + public override async Task<(object? value, uint status)> ReadAsync( + FocasAddress address, FocasDataType type, CancellationToken ct) + { + await Task.Delay(200, ct).ConfigureAwait(false); + if (!IsConnected) throw new ObjectDisposedException(nameof(SlowReadClient), "disposed mid-read"); + return ((sbyte)5, FocasStatusMapper.Good); + } + } + + /// + /// #740 / R07:L6 — the recycle loop disposed the device client with no gate, so a recycle tick landing + /// mid-read killed that read (BadCommunicationError + Degraded, every interval). The recycle now takes the + /// device's connect gate and lets the in-flight read finish before disposing the client it detached. + /// + [Fact] + public async Task Recycle_tick_landing_mid_read_lets_the_read_finish() + { + var factory = new FakeFocasClientFactory { Customise = () => new SlowReadClient() }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions("focas://10.0.0.5:8193")], + RawTags = [FocasRawTags.Entry(new FocasTagDefinition("R", "focas://10.0.0.5:8193", "R100", FocasDataType.Byte))], + Probe = new FocasProbeOptions { Enabled = false }, + HandleRecycle = new FocasHandleRecycleOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(50) }, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + // Each 200 ms read spans at least three recycle ticks. + for (var i = 0; i < 3; i++) + { + var snap = (await drv.ReadAsync(["R"], CancellationToken.None))[0]; + snap.StatusCode.ShouldBe(FocasStatusMapper.Good, $"read {i} was killed by a recycle tick"); + snap.Value.ShouldBe((sbyte)5); + } + drv.GetHealth().State.ShouldBe(Core.Abstractions.DriverState.Healthy); + + // Positive control: the recycle really ran — the first client was disposed and a fresh one opened. + await WaitFor(() => factory.Clients[0].DisposeCount > 0, TimeSpan.FromSeconds(3)); + factory.Clients[0].DisposeCount.ShouldBeGreaterThan(0); + factory.Clients.Count.ShouldBeGreaterThan(1); + await drv.ShutdownAsync(CancellationToken.None); + factory.Clients.ShouldAllBe(c => !c.IsConnected, "no client opened around a recycle may be orphaned"); + } + + /// DisposeWhenIdleAsync drains the in-flight op, then disposes; a later op is refused, not run on a dead socket. + [Fact] + public async Task DisposeWhenIdle_drains_the_in_flight_op_then_refuses_new_ones() + { + var inner = new SlowReadClient(); + await inner.ConnectAsync(new FocasHostAddress("10.0.0.5", 8193), TimeSpan.FromSeconds(1), CancellationToken.None); + var sync = new SynchronizedFocasClient(inner, TimeSpan.FromSeconds(5)); + var addr = FocasAddress.TryParse("R100")!; + + var read = sync.ReadAsync(addr, FocasDataType.Byte, CancellationToken.None); + await Task.Delay(20); + sync.PendingOperations.ShouldBe(1); + var dispose = sync.DisposeWhenIdleAsync(TimeSpan.FromSeconds(5)); + + (await read).status.ShouldBe(FocasStatusMapper.Good); + await dispose; + inner.DisposeCount.ShouldBe(1); + await Should.ThrowAsync(() => sync.ReadAsync(addr, FocasDataType.Byte, CancellationToken.None)); + } + private static async Task WaitFor(Func pred, TimeSpan timeout) { var deadline = DateTime.UtcNow + timeout;