fix(opcuaclient): shutdown cancels session work before the gate wait; dispose skips a redundant nested shutdown (review r2 R2-1, R2-2)
- R2-1: the ConditionRefresh sweeps (RefreshRequired, post-outage recovery) run under the session's work token, and ShutdownAsync cancels session work first thing - so a sweep holding the subscription gate against a stalling upstream no longer eats the whole teardown budget the deletes and close need. - R2-2: DisposeAsync skips its nested ShutdownAsync when a shutdown already started for the current lifecycle generation (nothing initialised since), so PostStop's dispose does not re-wait the gate past its budget. Test: with the gate held, the shutdown waits (positive control) and the dispose does not wait again; red with the skip removed. Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -1234,6 +1234,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
using var budget = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
using var budget = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||||
budget.CancelAfter(TeardownBudget);
|
budget.CancelAfter(TeardownBudget);
|
||||||
|
|
||||||
|
// r2 R2-1: cancel the session's background work FIRST — a ConditionRefresh sweep holding the subscription gate
|
||||||
|
// against a stalling upstream would otherwise make the gate wait below spend the whole wire budget, leaving the
|
||||||
|
// deletes and the close none. (ReleaseSessionAsync cancels it again; that is idempotent.)
|
||||||
|
CancelSessionWork();
|
||||||
|
|
||||||
|
// r2 R2-2: a later DisposeAsync for this same lifecycle skips its nested shutdown.
|
||||||
|
Interlocked.Exchange(ref _shutdownStartedGeneration, generation);
|
||||||
|
|
||||||
// r1 N-2: subscription management is held off for the whole teardown. A subscribe already in flight finishes
|
// r1 N-2: subscription management is held off for the whole teardown. A subscribe already in flight finishes
|
||||||
// first (bounded by the budget) and is enumerated below; one that arrives during the delete phase waits and
|
// first (bounded by the budget) and is enumerated below; one that arrives during the delete phase waits and
|
||||||
// then finds the session released, instead of registering a subscription on the closing session after the
|
// then finds the session released, instead of registering a subscription on the closing session after the
|
||||||
@@ -2281,6 +2289,9 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
/// </summary>
|
/// </summary>
|
||||||
private long _lifecycleGeneration;
|
private long _lifecycleGeneration;
|
||||||
|
|
||||||
|
/// <summary>The lifecycle generation the last <see cref="ShutdownAsync"/> started under (r2 R2-2).</summary>
|
||||||
|
private long _shutdownStartedGeneration = -1;
|
||||||
|
|
||||||
/// <summary>Test seam: the current lifecycle generation.</summary>
|
/// <summary>Test seam: the current lifecycle generation.</summary>
|
||||||
internal long LifecycleGenerationForTest => Volatile.Read(ref _lifecycleGeneration);
|
internal long LifecycleGenerationForTest => Volatile.Read(ref _lifecycleGeneration);
|
||||||
|
|
||||||
@@ -2700,7 +2711,9 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
if (IsRefreshRequired(fields))
|
if (IsRefreshRequired(fields))
|
||||||
{
|
{
|
||||||
// Part 9 §5.11.7: the server lost events (queue overflow / restart) and asks for a refresh.
|
// Part 9 §5.11.7: the server lost events (queue overflow / restart) and asks for a refresh.
|
||||||
_ = Task.Run(() => RefreshAllAlarmSubscriptionsAsync(CancellationToken.None));
|
// r2 R2-1: bound to the session's work token, which a shutdown cancels first thing.
|
||||||
|
var sessionWork = _sessionWorkCts.Token;
|
||||||
|
_ = Task.Run(() => RefreshAllAlarmSubscriptionsAsync(sessionWork));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2821,6 +2834,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
await _subscriptionGate.WaitAsync(cancellationToken).ConfigureAwait(false);
|
await _subscriptionGate.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
catch (ObjectDisposedException) { return; }
|
catch (ObjectDisposedException) { return; }
|
||||||
|
catch (OperationCanceledException) { return; /* the session's work was cancelled (shutdown / re-init) */ }
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
var session = Session;
|
var session = Session;
|
||||||
@@ -3418,7 +3432,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
if (!Current()) return;
|
if (!Current()) return;
|
||||||
AnnotateAlarmQuality(StatusGood, Clock.GetUtcNow().UtcDateTime);
|
AnnotateAlarmQuality(StatusGood, Clock.GetUtcNow().UtcDateTime);
|
||||||
if (!_alarmSubscriptions.IsEmpty)
|
if (!_alarmSubscriptions.IsEmpty)
|
||||||
await RefreshAllAlarmSubscriptionsAsync(CancellationToken.None).ConfigureAwait(false);
|
await RefreshAllAlarmSubscriptionsAsync(_sessionWorkCts.Token).ConfigureAwait(false);
|
||||||
}
|
}
|
||||||
catch (Exception ex)
|
catch (Exception ex)
|
||||||
{
|
{
|
||||||
@@ -3645,8 +3659,14 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
|||||||
{
|
{
|
||||||
if (_disposed) return;
|
if (_disposed) return;
|
||||||
_disposed = true;
|
_disposed = true;
|
||||||
try { await ShutdownAsync(CancellationToken.None).ConfigureAwait(false); }
|
// r2 R2-2: the host's PostStop runs ShutdownAsync then DisposeAsync under one budget. When nothing has happened
|
||||||
catch { /* disposal is best-effort */ }
|
// since that shutdown started (no init since), a nested shutdown would only re-wait the gate the first one may
|
||||||
|
// still hold and overrun the dispose bound — the running / finished shutdown already covers the teardown.
|
||||||
|
if (Volatile.Read(ref _shutdownStartedGeneration) != Volatile.Read(ref _lifecycleGeneration))
|
||||||
|
{
|
||||||
|
try { await ShutdownAsync(CancellationToken.None).ConfigureAwait(false); }
|
||||||
|
catch { /* disposal is best-effort */ }
|
||||||
|
}
|
||||||
|
|
||||||
// Detach the cert-validation handler.
|
// Detach the cert-validation handler.
|
||||||
if (_certValidationHandler is not null && _certValidatorRef is not null)
|
if (_certValidationHandler is not null && _certValidatorRef is not null)
|
||||||
|
|||||||
+21
@@ -257,4 +257,25 @@ public sealed class OpcUaClientLifecycleGenerationTests
|
|||||||
session.Verify(s => s.AddSubscription(It.IsAny<Subscription>()), Times.Once,
|
session.Verify(s => s.AddSubscription(It.IsAny<Subscription>()), Times.Once,
|
||||||
"the late subscribe must not have created anything on the closing session");
|
"the late subscribe must not have created anything on the closing session");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// r2 R2-2: PostStop runs ShutdownAsync then DisposeAsync. With subscription management contended (a subscribe
|
||||||
|
/// still holding the gate), the shutdown spends its budget waiting on the gate; the dispose that follows must not
|
||||||
|
/// re-run a nested shutdown and wait all over again. Positive control: the shutdown itself did wait.
|
||||||
|
/// </summary>
|
||||||
|
[Fact]
|
||||||
|
public async Task Dispose_after_a_shutdown_of_the_same_lifecycle_does_not_rewait_the_gate()
|
||||||
|
{
|
||||||
|
var drv = new OpcUaClientDriver(Options(), "opcua-r22") { TeardownBudget = TimeSpan.FromSeconds(3) };
|
||||||
|
drv.SetSessionForTest(NewSession().Object);
|
||||||
|
await drv.SubscriptionGate.WaitAsync(TestContext.Current.CancellationToken); // a "subscribe" that never finishes
|
||||||
|
|
||||||
|
var sw = Stopwatch.StartNew();
|
||||||
|
await drv.ShutdownAsync(TestContext.Current.CancellationToken);
|
||||||
|
sw.Elapsed.ShouldBeGreaterThan(TimeSpan.FromSeconds(2), "positive control: the shutdown waited on the held gate");
|
||||||
|
|
||||||
|
sw.Restart();
|
||||||
|
await drv.DisposeAsync();
|
||||||
|
sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(2), "the dispose must not wait the 3 s gate budget a second time");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user