fix(drivers): join FOCAS and AbLegacy background loops on shutdown (#602 STAB-23/6)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -229,7 +229,7 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
|||||||
{
|
{
|
||||||
state.ProbeCts = new CancellationTokenSource();
|
state.ProbeCts = new CancellationTokenSource();
|
||||||
var ct = state.ProbeCts.Token;
|
var ct = state.ProbeCts.Token;
|
||||||
_ = Task.Run(() => ProbeLoopAsync(state, ct), ct);
|
state.ProbeTask = Task.Run(() => ProbeLoopAsync(state, ct), ct);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
_health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null);
|
_health = new DriverHealth(DriverState.Healthy, DateTime.UtcNow, null);
|
||||||
@@ -245,10 +245,13 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
|||||||
// handles alive. Mirrors the body of ShutdownAsync without awaiting the poll engine
|
// handles alive. Mirrors the body of ShutdownAsync without awaiting the poll engine
|
||||||
// (nothing has been subscribed yet at init time).
|
// (nothing has been subscribed yet at init time).
|
||||||
foreach (var state in _devices.Values)
|
foreach (var state in _devices.Values)
|
||||||
{
|
|
||||||
try { state.ProbeCts?.Cancel(); } catch { }
|
try { state.ProbeCts?.Cancel(); } catch { }
|
||||||
|
JoinProbeLoopsSync();
|
||||||
|
foreach (var state in _devices.Values)
|
||||||
|
{
|
||||||
state.ProbeCts?.Dispose();
|
state.ProbeCts?.Dispose();
|
||||||
state.ProbeCts = null;
|
state.ProbeCts = null;
|
||||||
|
state.ProbeTask = null;
|
||||||
state.DisposeRuntimes();
|
state.DisposeRuntimes();
|
||||||
}
|
}
|
||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
@@ -266,15 +269,45 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
|||||||
await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false);
|
await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>Upper bound on how long shutdown waits for the probe loops to observe cancellation.</summary>
|
||||||
|
internal static readonly TimeSpan LoopJoinTimeout = TimeSpan.FromSeconds(5);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Synchronous bounded join of the (already-cancelled) probe loops for the sync teardown paths.
|
||||||
|
/// The loop body uses <c>ConfigureAwait(false)</c> throughout, so blocking here cannot deadlock.
|
||||||
|
/// </summary>
|
||||||
|
private void JoinProbeLoopsSync()
|
||||||
|
{
|
||||||
|
var loops = _devices.Values.Select(d => d.ProbeTask).OfType<Task>().ToArray();
|
||||||
|
if (loops.Length == 0) return;
|
||||||
|
try { Task.WaitAll(loops, LoopJoinTimeout); }
|
||||||
|
catch (AggregateException) { /* loops observe cancellation; a fault is not actionable at teardown */ }
|
||||||
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public async Task ShutdownAsync(CancellationToken cancellationToken)
|
public async Task ShutdownAsync(CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
await _poll.DisposeAsync().ConfigureAwait(false);
|
await _poll.DisposeAsync().ConfigureAwait(false);
|
||||||
|
// STAB-6: cancel the probe loops, JOIN them (bounded), then dispose the CTSs and runtimes.
|
||||||
|
// The loop used to be fire-and-forget, so one parked in a probe-runtime create/read could
|
||||||
|
// outlive shutdown and keep a libplctag handle alive past DisposeRuntimes.
|
||||||
|
foreach (var state in _devices.Values)
|
||||||
|
try { state.ProbeCts?.Cancel(); } catch { }
|
||||||
|
var loops = _devices.Values.Select(d => d.ProbeTask).OfType<Task>().ToList();
|
||||||
|
if (loops.Count > 0)
|
||||||
|
{
|
||||||
|
var all = Task.WhenAll(loops);
|
||||||
|
var finished = await Task.WhenAny(all, Task.Delay(LoopJoinTimeout, CancellationToken.None)).ConfigureAwait(false);
|
||||||
|
if (finished != all)
|
||||||
|
_logger.LogWarning(
|
||||||
|
"AbLegacy shutdown: {Count} probe loop(s) did not stop within {Timeout}; proceeding with teardown. Driver={DriverInstanceId}",
|
||||||
|
loops.Count, LoopJoinTimeout, _driverInstanceId);
|
||||||
|
}
|
||||||
foreach (var state in _devices.Values)
|
foreach (var state in _devices.Values)
|
||||||
{
|
{
|
||||||
try { state.ProbeCts?.Cancel(); } catch { }
|
|
||||||
state.ProbeCts?.Dispose();
|
state.ProbeCts?.Dispose();
|
||||||
state.ProbeCts = null;
|
state.ProbeCts = null;
|
||||||
|
state.ProbeTask = null;
|
||||||
state.DisposeRuntimes();
|
state.DisposeRuntimes();
|
||||||
}
|
}
|
||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
@@ -892,10 +925,13 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
|||||||
// can't poison the rest of the disposal chain.
|
// can't poison the rest of the disposal chain.
|
||||||
try { _poll.DisposeAsync().ConfigureAwait(false).GetAwaiter().GetResult(); } catch { }
|
try { _poll.DisposeAsync().ConfigureAwait(false).GetAwaiter().GetResult(); } catch { }
|
||||||
foreach (var state in _devices.Values)
|
foreach (var state in _devices.Values)
|
||||||
{
|
|
||||||
try { state.ProbeCts?.Cancel(); } catch { }
|
try { state.ProbeCts?.Cancel(); } catch { }
|
||||||
|
JoinProbeLoopsSync();
|
||||||
|
foreach (var state in _devices.Values)
|
||||||
|
{
|
||||||
state.ProbeCts?.Dispose();
|
state.ProbeCts?.Dispose();
|
||||||
state.ProbeCts = null;
|
state.ProbeCts = null;
|
||||||
|
state.ProbeTask = null;
|
||||||
state.DisposeRuntimes();
|
state.DisposeRuntimes();
|
||||||
}
|
}
|
||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
@@ -1021,6 +1057,11 @@ public sealed class AbLegacyDriver : IDriver, IReadable, IWritable, ITagDiscover
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
public CancellationTokenSource? ProbeCts { get; set; }
|
public CancellationTokenSource? ProbeCts { get; set; }
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Gets or sets the probe loop task, retained so shutdown can join it (STAB-6).
|
||||||
|
/// </summary>
|
||||||
|
public Task? ProbeTask { get; set; }
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets or sets a value indicating whether the probe has been initialized.
|
/// Gets or sets a value indicating whether the probe has been initialized.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|||||||
@@ -277,7 +277,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
{
|
{
|
||||||
state.ProbeCts = new CancellationTokenSource();
|
state.ProbeCts = new CancellationTokenSource();
|
||||||
var ct = state.ProbeCts.Token;
|
var ct = state.ProbeCts.Token;
|
||||||
_ = Task.Run(() => ProbeLoopAsync(state, ct), ct);
|
state.ProbeTask = Task.Run(() => ProbeLoopAsync(state, ct), ct);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -287,7 +287,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
{
|
{
|
||||||
state.RecycleCts = new CancellationTokenSource();
|
state.RecycleCts = new CancellationTokenSource();
|
||||||
var ct = state.RecycleCts.Token;
|
var ct = state.RecycleCts.Token;
|
||||||
_ = Task.Run(() => RecycleLoopAsync(state, ct), ct);
|
state.RecycleTask = Task.Run(() => RecycleLoopAsync(state, ct), ct);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -305,7 +305,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
{
|
{
|
||||||
state.FixedTreeCts = new CancellationTokenSource();
|
state.FixedTreeCts = new CancellationTokenSource();
|
||||||
var ct = state.FixedTreeCts.Token;
|
var ct = state.FixedTreeCts.Token;
|
||||||
_ = Task.Run(() => FixedTreeLoopAsync(state, ct), ct);
|
state.FixedTreeTask = Task.Run(() => FixedTreeLoopAsync(state, ct), ct);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -326,6 +326,9 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false);
|
await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>Upper bound on how long shutdown waits for the background loops to observe cancellation.</summary>
|
||||||
|
internal static readonly TimeSpan LoopJoinTimeout = TimeSpan.FromSeconds(5);
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
public async Task ShutdownAsync(CancellationToken cancellationToken)
|
public async Task ShutdownAsync(CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
@@ -335,22 +338,43 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
await proj.DisposeAsync().ConfigureAwait(false);
|
await proj.DisposeAsync().ConfigureAwait(false);
|
||||||
_alarmProjection = null;
|
_alarmProjection = null;
|
||||||
}
|
}
|
||||||
|
// STAB-23: cancel every loop first, JOIN them (bounded), and only then dispose the CTSs and the
|
||||||
|
// client. Fire-and-forget loops used to outlive shutdown: one parked in EnsureConnectedAsync could
|
||||||
|
// finish its connect AFTER DisposeClient and leave a live socket on a shut-down driver.
|
||||||
|
var loops = new List<Task>();
|
||||||
foreach (var state in _devices.Values)
|
foreach (var state in _devices.Values)
|
||||||
{
|
{
|
||||||
// Cancel-then-dispose can race in tight shutdown loops; swallowing is intentional
|
// Cancel-then-dispose can race in tight shutdown loops; swallowing is intentional
|
||||||
// but we now log the cause so a noisy shutdown leaves a Debug trace.
|
// but we now log the cause so a noisy shutdown leaves a Debug trace.
|
||||||
try { state.ProbeCts?.Cancel(); }
|
try { state.ProbeCts?.Cancel(); }
|
||||||
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling probe CTS for {Host} failed", state.Options.HostAddress); }
|
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling probe CTS for {Host} failed", state.Options.HostAddress); }
|
||||||
state.ProbeCts?.Dispose();
|
|
||||||
state.ProbeCts = null;
|
|
||||||
try { state.RecycleCts?.Cancel(); }
|
try { state.RecycleCts?.Cancel(); }
|
||||||
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling recycle CTS for {Host} failed", state.Options.HostAddress); }
|
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling recycle CTS for {Host} failed", state.Options.HostAddress); }
|
||||||
state.RecycleCts?.Dispose();
|
|
||||||
state.RecycleCts = null;
|
|
||||||
try { state.FixedTreeCts?.Cancel(); }
|
try { state.FixedTreeCts?.Cancel(); }
|
||||||
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling fixed-tree CTS for {Host} failed", state.Options.HostAddress); }
|
catch (Exception ex) { _logger.LogDebug(ex, "Cancelling fixed-tree CTS for {Host} failed", state.Options.HostAddress); }
|
||||||
|
if (state.ProbeTask is { } pt) loops.Add(pt);
|
||||||
|
if (state.RecycleTask is { } rt) loops.Add(rt);
|
||||||
|
if (state.FixedTreeTask is { } ft) loops.Add(ft);
|
||||||
|
}
|
||||||
|
if (loops.Count > 0)
|
||||||
|
{
|
||||||
|
var all = Task.WhenAll(loops);
|
||||||
|
var finished = await Task.WhenAny(all, Task.Delay(LoopJoinTimeout, CancellationToken.None)).ConfigureAwait(false);
|
||||||
|
if (finished != all)
|
||||||
|
_logger.LogWarning("FOCAS shutdown: {Count} background loop(s) did not stop within {Timeout}; proceeding with teardown",
|
||||||
|
loops.Count, LoopJoinTimeout);
|
||||||
|
}
|
||||||
|
foreach (var state in _devices.Values)
|
||||||
|
{
|
||||||
|
state.ProbeCts?.Dispose();
|
||||||
|
state.ProbeCts = null;
|
||||||
|
state.ProbeTask = null;
|
||||||
|
state.RecycleCts?.Dispose();
|
||||||
|
state.RecycleCts = null;
|
||||||
|
state.RecycleTask = null;
|
||||||
state.FixedTreeCts?.Dispose();
|
state.FixedTreeCts?.Dispose();
|
||||||
state.FixedTreeCts = null;
|
state.FixedTreeCts = null;
|
||||||
|
state.FixedTreeTask = null;
|
||||||
state.DisposeClient();
|
state.DisposeClient();
|
||||||
}
|
}
|
||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
@@ -1575,6 +1599,12 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
public CancellationTokenSource? RecycleCts { get; set; }
|
public CancellationTokenSource? RecycleCts { get; set; }
|
||||||
/// <summary>Gets or sets the cancellation token source for the fixed-tree loop.</summary>
|
/// <summary>Gets or sets the cancellation token source for the fixed-tree loop.</summary>
|
||||||
public CancellationTokenSource? FixedTreeCts { get; set; }
|
public CancellationTokenSource? FixedTreeCts { get; set; }
|
||||||
|
/// <summary>The probe loop task, retained so shutdown can join it (STAB-23).</summary>
|
||||||
|
public Task? ProbeTask { get; set; }
|
||||||
|
/// <summary>The recycle loop task, retained so shutdown can join it (STAB-23).</summary>
|
||||||
|
public Task? RecycleTask { get; set; }
|
||||||
|
/// <summary>The fixed-tree loop task, retained so shutdown can join it (STAB-23).</summary>
|
||||||
|
public Task? FixedTreeTask { get; set; }
|
||||||
/// <summary>Gets or sets the fixed-tree cache for this device.</summary>
|
/// <summary>Gets or sets the fixed-tree cache for this device.</summary>
|
||||||
public FocasFixedTreeCache? FixedTreeCache { get; set; }
|
public FocasFixedTreeCache? FixedTreeCache { get; set; }
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,54 @@
|
|||||||
|
using Shouldly;
|
||||||
|
using Xunit;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.PlcFamilies;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Driver.AbLegacy.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// STAB-6 / #602 — the probe loop was fire-and-forget, so shutdown cancelled and disposed while the
|
||||||
|
/// loop could still be parked in a probe-runtime call that ignores cancellation, leaving its
|
||||||
|
/// libplctag handle alive past teardown. Shutdown now joins the loop (bounded) first.
|
||||||
|
/// </summary>
|
||||||
|
[Trait("Category", "Unit")]
|
||||||
|
public sealed class AbLegacyShutdownJoinTests
|
||||||
|
{
|
||||||
|
private sealed class ParkedInitTag(
|
||||||
|
AbLegacyTagCreateParams p, TaskCompletionSource entered, TaskCompletionSource release) : FakeAbLegacyTag(p)
|
||||||
|
{
|
||||||
|
public override async Task InitializeAsync(CancellationToken ct)
|
||||||
|
{
|
||||||
|
entered.TrySetResult();
|
||||||
|
await release.Task.ConfigureAwait(false); // deliberately ignores ct
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Shutdown blocks on a probe loop parked in runtime init, then the loop disposes its probe runtime.</summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task Shutdown_WaitsForProbeLoopParkedInInit_ThenProbeRuntimeIsDisposed()
|
||||||
|
{
|
||||||
|
var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var factory = new FakeAbLegacyTagFactory
|
||||||
|
{
|
||||||
|
Customise = p => new ParkedInitTag(p, entered, release),
|
||||||
|
};
|
||||||
|
var drv = new AbLegacyDriver(new AbLegacyDriverOptions
|
||||||
|
{
|
||||||
|
Devices = [new AbLegacyDeviceOptions("ab://10.0.0.5/1,0", AbLegacyPlcFamily.Slc500)],
|
||||||
|
Probe = new AbLegacyProbeOptions { Enabled = true, Interval = TimeSpan.FromMilliseconds(50), ProbeAddress = "S:0" },
|
||||||
|
}, "drv-join", factory);
|
||||||
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||||
|
await entered.Task.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
var shutdown = drv.ShutdownAsync(CancellationToken.None);
|
||||||
|
|
||||||
|
await Task.Delay(300);
|
||||||
|
shutdown.IsCompleted.ShouldBeFalse("shutdown must join the probe loop parked in runtime init");
|
||||||
|
|
||||||
|
// Positive control: releasing the parked call lets shutdown finish, with the probe runtime released.
|
||||||
|
release.SetResult();
|
||||||
|
await shutdown.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
factory.Tags["S:0"].Disposed.ShouldBeTrue();
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -0,0 +1,57 @@
|
|||||||
|
using Shouldly;
|
||||||
|
using Xunit;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// STAB-23 / #602 — the probe / recycle / fixed-tree loops were fire-and-forget, so shutdown
|
||||||
|
/// cancelled and disposed while a loop could still be parked inside a connect that ignores
|
||||||
|
/// cancellation, and that connect then completed against a shut-down driver. Shutdown now joins
|
||||||
|
/// the loops (bounded) before disposing the client.
|
||||||
|
/// </summary>
|
||||||
|
[Trait("Category", "Unit")]
|
||||||
|
public sealed class FocasShutdownJoinTests
|
||||||
|
{
|
||||||
|
/// <summary>A client whose connect parks until released and deliberately ignores the cancellation token.</summary>
|
||||||
|
private sealed class ParkedConnectClient(TaskCompletionSource entered, TaskCompletionSource release) : FakeFocasClient
|
||||||
|
{
|
||||||
|
public override async Task ConnectAsync(FocasHostAddress address, TimeSpan timeout, CancellationToken ct)
|
||||||
|
{
|
||||||
|
entered.TrySetResult();
|
||||||
|
await release.Task.ConfigureAwait(false);
|
||||||
|
await base.ConnectAsync(address, timeout, CancellationToken.None).ConfigureAwait(false);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>Shutdown blocks on a probe loop parked in connect, then disposes the client that connect produced.</summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task Shutdown_WaitsForLoopParkedInConnect_ThenDisposesItsClient()
|
||||||
|
{
|
||||||
|
var entered = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var release = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
|
||||||
|
var factory = new FakeFocasClientFactory { Customise = () => new ParkedConnectClient(entered, release) };
|
||||||
|
var options = 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 = true, Interval = TimeSpan.FromMilliseconds(50), Timeout = TimeSpan.FromSeconds(10) },
|
||||||
|
};
|
||||||
|
var drv = new FocasDriver(options, "focas-1", factory);
|
||||||
|
await drv.InitializeAsync("{}", CancellationToken.None);
|
||||||
|
await entered.Task.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
var shutdown = drv.ShutdownAsync(CancellationToken.None);
|
||||||
|
|
||||||
|
await Task.Delay(300);
|
||||||
|
shutdown.IsCompleted.ShouldBeFalse("shutdown must join the probe loop parked in connect");
|
||||||
|
|
||||||
|
// Positive control: releasing the parked connect lets shutdown finish.
|
||||||
|
release.SetResult();
|
||||||
|
await shutdown.WaitAsync(TimeSpan.FromSeconds(5));
|
||||||
|
|
||||||
|
factory.Clients.ShouldNotBeEmpty();
|
||||||
|
factory.Clients.ShouldAllBe(c => c.DisposeCount >= 1);
|
||||||
|
factory.Clients.ShouldAllBe(c => !c.IsConnected);
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user