fix(opcuaclient): review r0 minors m1-m10
- m1: the form hint now says ProbeTimeoutSeconds can only shorten Test Connect's 10 s budget (passing the configured value as the button's budget is shared code - follow-up). - m2: a superseded init no longer overwrites _options (generation-checked under _probeLock) or the cert-validation handler (handed back and published with the session; detached when not installed; a previous one detached on replace). - m3: an ack none of whose condition ids resolves throws BadNodeIdUnknown + Warning. - m4: an ack with no reply (transport failure / non-caller timeout) degrades. - m5: no session before the gate is the same ServiceResultException(BadNotConnected). - m6: the caller-cancelled-write deviation is documented. - m7: a reply during a published outage advances the stamp but does not write Healthy; the keep-alive recovery ends the outage. - m8: test for a non-caller OperationCanceledException write -> Degraded. - m9: the browse session owns and disposes the user certificate it loaded (OpcUaUserIdentityFactory.LoadCertificate; a key-less cert is disposed before the throw); AdminUI-host certificate path documented. - m10: a subscribe overtaken by a shutdown / re-init removes its half-built subscription instead of registering it (data and alarm). - Sync Dispose drains each gate for 500 ms (was 2 s). Refs #743 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
@@ -26,6 +26,7 @@ out-of-process isolation.
|
||||
|---------|--------|------|
|
||||
| `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/` | net10.0 | In-process driver — session lifetime, read / write / subscribe / alarm / history passthrough |
|
||||
| `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Browser/` | net10.0 | `IDriverBrowser` — live address-picker browse used by the AdminUI. Its side-panel `AttributesAsync` reads a variable's DataType / ValueRank / UserAccessLevel and maps them through the same `UpstreamAttributeMapping` discovery uses; it authenticates Anonymous, Username **or Certificate** through the driver's own `OpcUaUserIdentityFactory`, resolving `secret:` credentials through the host's secret store (fail-closed) — review 2026-10-21 O-5 (it was a stub, and Certificate was refused) |
|
||||
| | | ⚠️ Browse Certificate auth reads `UserCertificatePath` on the **AdminUI host's** filesystem, which may differ from the driver node's; the browse session disposes the certificate it loaded when it closes. |
|
||||
| `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Session/` | net10.0 | The SDK-typed layer the driver AND the browser share: `NamespaceMap`, `UpstreamAttributeMapping`, `OpcUaUserIdentityFactory` |
|
||||
| `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Contracts/` | net10.0 | Config records + enums bound from `DriverConfig` JSON |
|
||||
|
||||
@@ -98,7 +99,13 @@ The session keep-alive (`KeepAliveInterval`, default 5 s) is the liveness signal
|
||||
only the stamp, so a subscription-only driver stayed `Degraded` until its next re-initialise).
|
||||
- **Read / write / ack responses are replies.** Any service response — per-node refusals such as `BadNotWritable`
|
||||
included — restores `Healthy` and advances the stamp; only a call that got **no** response (transport failure,
|
||||
timeout) writes `Degraded` with the stamp preserved. A write the caller cancelled touches no health.
|
||||
timeout — including an SDK request timeout surfacing as a cancellation the caller did not cause) writes `Degraded`
|
||||
with the stamp preserved; acks follow the same rule. **Deliberate deviation:** a call the *caller* cancelled touches
|
||||
no health — including a write the host cancelled at its own write deadline against a hung upstream; the session
|
||||
keep-alive owns liveness and reports that outage itself. A reply that was in flight across a bad keep-alive advances
|
||||
the stamp but does not write `Healthy` while the outage is published — the keep-alive recovery ends the outage.
|
||||
An ack none of whose condition ids resolves sends nothing and fails `BadNodeIdUnknown`; with no session an ack fails
|
||||
`BadNotConnected` (before or inside the gate alike).
|
||||
- **Bad keep-alive** ⇒ health `Degraded` (`LastError = "keep-alive: <status>"`, read stamp preserved), host status
|
||||
`Stopped`, and — **once per outage**, not per keep-alive — every subscribed ref is published `BadNoCommunication`
|
||||
with its last value retained and **source + server timestamp = the detection time** (live-gate F1: stamped with the
|
||||
|
||||
+7
-1
@@ -17,6 +17,7 @@ internal sealed class OpcUaClientBrowseSession : IBrowseSession
|
||||
private readonly NodeId _rootNodeId;
|
||||
private readonly SemaphoreSlim _gate = new(1, 1);
|
||||
private volatile bool _disposed;
|
||||
private readonly IDisposable? _ownedCredential;
|
||||
|
||||
/// <summary>
|
||||
/// Construct a browse session bound to an already-connected <paramref name="session"/>.
|
||||
@@ -25,8 +26,10 @@ internal sealed class OpcUaClientBrowseSession : IBrowseSession
|
||||
/// <param name="nsMap">Namespace snapshot taken at connect time; used to render outbound
|
||||
/// NodeIds in the server-stable <c>nsu=…</c> form.</param>
|
||||
/// <param name="rootNodeId">The node under which <see cref="RootAsync"/> browses one level.</param>
|
||||
internal OpcUaClientBrowseSession(ISession session, NamespaceMap nsMap, NodeId rootNodeId)
|
||||
/// <param name="ownedCredential">A credential (the user certificate) this session owns and disposes with itself.</param>
|
||||
internal OpcUaClientBrowseSession(ISession session, NamespaceMap nsMap, NodeId rootNodeId, IDisposable? ownedCredential = null)
|
||||
{
|
||||
_ownedCredential = ownedCredential;
|
||||
_session = session;
|
||||
_nsMap = nsMap;
|
||||
_rootNodeId = rootNodeId;
|
||||
@@ -242,5 +245,8 @@ internal sealed class OpcUaClientBrowseSession : IBrowseSession
|
||||
|
||||
try { _gate.Dispose(); }
|
||||
catch { /* best-effort */ }
|
||||
|
||||
try { _ownedCredential?.Dispose(); }
|
||||
catch { /* best-effort */ }
|
||||
}
|
||||
}
|
||||
|
||||
+73
-43
@@ -69,55 +69,66 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
|
||||
// R06:O-5 — Certificate auth is supported: the identity is built (and any secret: credential resolved) by the
|
||||
// same factory the runtime driver uses, BEFORE any network I/O, so a bad certificate path / password fails fast
|
||||
// with its own message.
|
||||
var identity = await BuildBrowseUserIdentityAsync(opts, _secretResolver, cancellationToken).ConfigureAwait(false);
|
||||
|
||||
if (opts.AutoAcceptCertificates)
|
||||
_logger.LogWarning(
|
||||
"AdminUI browse session opens against {Endpoint} with form's AutoAcceptCertificates=true — " +
|
||||
"browse uses its own cert store and does NOT auto-accept; trust the cert via the runtime " +
|
||||
"driver's PKI store instead.",
|
||||
endpoint);
|
||||
|
||||
var appConfig = await BuildBrowseAppConfigurationAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
var perEndpointBudget = TimeSpan.FromSeconds(
|
||||
Math.Clamp(opts.PerEndpointConnectTimeout.TotalSeconds, 5, 30));
|
||||
using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||
connectCts.CancelAfter(perEndpointBudget);
|
||||
|
||||
var endpointDesc = await SelectEndpointAsync(
|
||||
appConfig, endpoint, opts.SecurityPolicy, opts.SecurityMode, connectCts.Token).ConfigureAwait(false);
|
||||
var endpointCfg = EndpointConfiguration.Create(appConfig);
|
||||
endpointCfg.OperationTimeout = (int)opts.Timeout.TotalMilliseconds;
|
||||
var configuredEndpoint = new ConfiguredEndpoint(null, endpointDesc, endpointCfg);
|
||||
|
||||
var session = await new DefaultSessionFactory(telemetry: null!).CreateAsync(
|
||||
appConfig,
|
||||
configuredEndpoint,
|
||||
updateBeforeConnect: false,
|
||||
sessionName: "OtOpcUa AdminUI Browse",
|
||||
(uint)opts.SessionTimeout.TotalMilliseconds,
|
||||
identity,
|
||||
preferredLocales: null,
|
||||
connectCts.Token).ConfigureAwait(false);
|
||||
var (identity, ownedCertificate) = await BuildBrowseCredentialAsync(opts, _secretResolver, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
|
||||
try
|
||||
{
|
||||
var nsMap = NamespaceMap.FromSession(session);
|
||||
var rootNodeId = string.IsNullOrEmpty(opts.BrowseRoot)
|
||||
? ObjectIds.ObjectsFolder
|
||||
: NodeId.Parse(session.MessageContext, opts.BrowseRoot);
|
||||
if (opts.AutoAcceptCertificates)
|
||||
_logger.LogWarning(
|
||||
"AdminUI browse session opens against {Endpoint} with form's AutoAcceptCertificates=true — " +
|
||||
"browse uses its own cert store and does NOT auto-accept; trust the cert via the runtime " +
|
||||
"driver's PKI store instead.",
|
||||
endpoint);
|
||||
|
||||
_logger.LogInformation(
|
||||
"AdminUI OPC UA browse session opened against {Endpoint} (policy {Policy}, mode {Mode})",
|
||||
endpoint, opts.SecurityPolicy, opts.SecurityMode);
|
||||
var appConfig = await BuildBrowseAppConfigurationAsync(cancellationToken).ConfigureAwait(false);
|
||||
|
||||
return new OpcUaClientBrowseSession(session, nsMap, rootNodeId);
|
||||
var perEndpointBudget = TimeSpan.FromSeconds(
|
||||
Math.Clamp(opts.PerEndpointConnectTimeout.TotalSeconds, 5, 30));
|
||||
using var connectCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
|
||||
connectCts.CancelAfter(perEndpointBudget);
|
||||
|
||||
var endpointDesc = await SelectEndpointAsync(
|
||||
appConfig, endpoint, opts.SecurityPolicy, opts.SecurityMode, connectCts.Token).ConfigureAwait(false);
|
||||
var endpointCfg = EndpointConfiguration.Create(appConfig);
|
||||
endpointCfg.OperationTimeout = (int)opts.Timeout.TotalMilliseconds;
|
||||
var configuredEndpoint = new ConfiguredEndpoint(null, endpointDesc, endpointCfg);
|
||||
|
||||
var session = await new DefaultSessionFactory(telemetry: null!).CreateAsync(
|
||||
appConfig,
|
||||
configuredEndpoint,
|
||||
updateBeforeConnect: false,
|
||||
sessionName: "OtOpcUa AdminUI Browse",
|
||||
(uint)opts.SessionTimeout.TotalMilliseconds,
|
||||
identity,
|
||||
preferredLocales: null,
|
||||
connectCts.Token).ConfigureAwait(false);
|
||||
|
||||
try
|
||||
{
|
||||
var nsMap = NamespaceMap.FromSession(session);
|
||||
var rootNodeId = string.IsNullOrEmpty(opts.BrowseRoot)
|
||||
? ObjectIds.ObjectsFolder
|
||||
: NodeId.Parse(session.MessageContext, opts.BrowseRoot);
|
||||
|
||||
_logger.LogInformation(
|
||||
"AdminUI OPC UA browse session opened against {Endpoint} (policy {Policy}, mode {Mode})",
|
||||
endpoint, opts.SecurityPolicy, opts.SecurityMode);
|
||||
|
||||
return new OpcUaClientBrowseSession(session, nsMap, rootNodeId, ownedCertificate);
|
||||
}
|
||||
catch
|
||||
{
|
||||
try { if (session is Session s) await s.CloseAsync().ConfigureAwait(false); } catch { /* best-effort */ }
|
||||
try { session.Dispose(); } catch { /* best-effort */ }
|
||||
throw;
|
||||
}
|
||||
}
|
||||
catch
|
||||
{
|
||||
try { if (session is Session s) await s.CloseAsync().ConfigureAwait(false); } catch { /* best-effort */ }
|
||||
try { session.Dispose(); } catch { /* best-effort */ }
|
||||
// Review m9: the certificate (and its private-key handle) is owned by the browse session once handed
|
||||
// over; any failure before that releases it here instead of leaving it to the GC.
|
||||
ownedCertificate?.Dispose();
|
||||
throw;
|
||||
}
|
||||
}
|
||||
@@ -192,10 +203,29 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
|
||||
/// <param name="ct">Cancellation token for the secret resolution.</param>
|
||||
/// <returns>The identity the browse session activates with.</returns>
|
||||
internal static async Task<UserIdentity> BuildBrowseUserIdentityAsync(
|
||||
OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct)
|
||||
OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct) =>
|
||||
(await BuildBrowseCredentialAsync(opts, secretResolver, ct).ConfigureAwait(false)).Identity;
|
||||
|
||||
/// <summary>
|
||||
/// As <see cref="BuildBrowseUserIdentityAsync"/>, also handing back the user certificate a Certificate-auth
|
||||
/// identity holds, so the browse session that uses it can dispose it (review m9: the browse opens one per picker
|
||||
/// session, and an undisposed <c>X509Certificate2</c> pins a private-key handle until GC).
|
||||
/// </summary>
|
||||
/// <param name="opts">Driver options carrying the form's auth fields.</param>
|
||||
/// <param name="secretResolver">The secret store, or null when none is available.</param>
|
||||
/// <param name="ct">Cancellation token for the secret resolution.</param>
|
||||
/// <returns>The identity, and the certificate it owns (null for Anonymous / Username).</returns>
|
||||
internal static async Task<(UserIdentity Identity, System.Security.Cryptography.X509Certificates.X509Certificate2? Certificate)>
|
||||
BuildBrowseCredentialAsync(OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct)
|
||||
{
|
||||
var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync(opts, secretResolver, ct).ConfigureAwait(false);
|
||||
return OpcUaUserIdentityFactory.Build(resolved);
|
||||
if (resolved.AuthType != OpcUaAuthType.Certificate)
|
||||
{
|
||||
return (OpcUaUserIdentityFactory.Build(resolved), null);
|
||||
}
|
||||
|
||||
var certificate = OpcUaUserIdentityFactory.LoadCertificate(resolved);
|
||||
return (new UserIdentity(certificate), certificate);
|
||||
}
|
||||
|
||||
/// <summary>Select the endpoint matching the requested SecurityPolicy + SecurityMode pair.</summary>
|
||||
|
||||
+13
-2
@@ -37,7 +37,15 @@ public static class OpcUaUserIdentityFactory
|
||||
/// </summary>
|
||||
/// <param name="options">Driver options containing certificate configuration.</param>
|
||||
/// <returns>The certificate-backed <see cref="UserIdentity"/>.</returns>
|
||||
public static UserIdentity FromCertificate(OpcUaClientDriverOptions options)
|
||||
public static UserIdentity FromCertificate(OpcUaClientDriverOptions options) => new(LoadCertificate(options));
|
||||
|
||||
/// <summary>
|
||||
/// Loads (and validates) the user-authentication certificate <see cref="FromCertificate"/> wraps. The caller owns
|
||||
/// the returned certificate — dispose it when the session that presented it is gone.
|
||||
/// </summary>
|
||||
/// <param name="options">Driver options containing certificate configuration.</param>
|
||||
/// <returns>The certificate, with its private key.</returns>
|
||||
public static System.Security.Cryptography.X509Certificates.X509Certificate2 LoadCertificate(OpcUaClientDriverOptions options)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(options.UserCertificatePath))
|
||||
throw new InvalidOperationException(
|
||||
@@ -53,11 +61,14 @@ public static class OpcUaUserIdentityFactory
|
||||
.LoadPkcs12FromFile(options.UserCertificatePath, options.UserCertificatePassword);
|
||||
|
||||
if (!cert.HasPrivateKey)
|
||||
{
|
||||
cert.Dispose();
|
||||
throw new InvalidOperationException(
|
||||
$"User certificate at '{options.UserCertificatePath}' has no private key — " +
|
||||
"the private key is required to sign the OPC UA user-token challenge at session activation.");
|
||||
}
|
||||
|
||||
return new UserIdentity(cert);
|
||||
return cert;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -473,6 +473,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
WriteLifecycleHealth(generation, _ => new DriverHealth(DriverState.Initializing, null, null));
|
||||
ISession? session = null;
|
||||
KeepAliveEventHandler? keepAliveHandler = null;
|
||||
var certValidation = new CertValidationHook();
|
||||
var installed = false;
|
||||
try
|
||||
{
|
||||
@@ -481,11 +482,18 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
// with — the endpoint, security policy and tag set were all frozen at construction, and only
|
||||
// secret rotation was picked up.
|
||||
if (HasConfigBody(driverConfigJson))
|
||||
_options = OpcUaClientDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson);
|
||||
{
|
||||
var parsed = OpcUaClientDriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson);
|
||||
// Review m2: a superseded init must not overwrite the options a newer one adopted.
|
||||
lock (_probeLock)
|
||||
{
|
||||
if (Volatile.Read(ref _lifecycleGeneration) == generation) _options = parsed;
|
||||
}
|
||||
}
|
||||
|
||||
ValidateTuning(_options);
|
||||
|
||||
var appConfig = await BuildApplicationConfigurationAsync(cancellationToken).ConfigureAwait(false);
|
||||
var appConfig = await BuildApplicationConfigurationAsync(certValidation, cancellationToken).ConfigureAwait(false);
|
||||
|
||||
// when a DiscoveryUrl is configured, FindServers/GetEndpoints on it first and try the
|
||||
// discovered URLs ahead of the static list. Non-fatal — an LDS outage must not block init.
|
||||
@@ -587,6 +595,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
Session = session;
|
||||
_keepAliveHandler = keepAliveHandler;
|
||||
_connectedEndpointUrl = connectedUrl;
|
||||
// Review m2: the cert-validation handler is published with the session — a previous one (a
|
||||
// re-initialise without shutdown) is detached first so it is not leaked on its validator.
|
||||
DetachCertValidation(_certValidatorRef, _certValidationHandler);
|
||||
_certValidationHandler = certValidation.Handler;
|
||||
_certValidatorRef = certValidation.Validator;
|
||||
installed = true;
|
||||
}
|
||||
}
|
||||
@@ -607,6 +620,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
// superseded check) does not orphan it and its keep-alive timer until GC.
|
||||
if (session is not null && !installed)
|
||||
await DiscardSessionAsync(session, keepAliveHandler).ConfigureAwait(false);
|
||||
if (!installed) DetachCertValidation(certValidation.Validator, certValidation.Handler);
|
||||
if (Volatile.Read(ref _lifecycleGeneration) == generation)
|
||||
{
|
||||
if (installed)
|
||||
@@ -618,6 +632,19 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The auto-accept certificate-validation handler one initialise attached, handed back to it (review m2).</summary>
|
||||
private sealed class CertValidationHook
|
||||
{
|
||||
public CertificateValidationEventHandler? Handler { get; set; }
|
||||
public CertificateValidator? Validator { get; set; }
|
||||
}
|
||||
|
||||
private static void DetachCertValidation(CertificateValidator? validator, CertificateValidationEventHandler? handler)
|
||||
{
|
||||
if (validator is null || handler is null) return;
|
||||
try { validator.CertificateValidation -= handler; } catch { /* best-effort */ }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Test seam: opens the session for one endpoint URL in place of the real discovery + session create, so
|
||||
/// the post-connect half of <see cref="InitializeAsync"/> (and its superseded / failure paths) can be driven
|
||||
@@ -672,7 +699,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
/// — so multiple driver instances in the same OtOpcUa server process share one
|
||||
/// certificate store without extra config.
|
||||
/// </summary>
|
||||
private async Task<ApplicationConfiguration> BuildApplicationConfigurationAsync(CancellationToken ct)
|
||||
/// <param name="certValidation">Receives the auto-accept handler this build attached (if any) — published by the
|
||||
/// caller only when its initialise is still current (review m2), detached otherwise.</param>
|
||||
/// <param name="ct">Cancellation token.</param>
|
||||
private async Task<ApplicationConfiguration> BuildApplicationConfigurationAsync(
|
||||
CertValidationHook certValidation, CancellationToken ct)
|
||||
{
|
||||
// The ITelemetryContext overload is the non-obsolete ctor; SDK warnings reach the driver's
|
||||
// logger when one was supplied.
|
||||
@@ -762,10 +793,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
e.Accept = true;
|
||||
};
|
||||
config.CertificateValidator.CertificateValidation += handler;
|
||||
// Store refs so ShutdownAsync + Dispose can detach the delegate and avoid
|
||||
// leaking a closure on a potentially process-shared validator.
|
||||
_certValidationHandler = handler;
|
||||
_certValidatorRef = config.CertificateValidator;
|
||||
// Handed back to the caller, which stores the refs (so ShutdownAsync + Dispose can detach the delegate)
|
||||
// only while its initialise is the current lifecycle generation.
|
||||
certValidation.Handler = handler;
|
||||
certValidation.Validator = config.CertificateValidator;
|
||||
}
|
||||
|
||||
// Ensure an application certificate exists. The SDK auto-generates one if missing.
|
||||
@@ -1856,11 +1887,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
TimestampsToReturn = TimestampsToReturn.Both,
|
||||
});
|
||||
|
||||
long subscribeGeneration = 0;
|
||||
await _subscriptionGate.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
// Re-read Session inside the critical section: a reconnect completing while we were
|
||||
// blocked on _gate has already swapped in the new session via a re-initialise.
|
||||
subscribeGeneration = Volatile.Read(ref _lifecycleGeneration);
|
||||
var session = Session
|
||||
?? throw new InvalidOperationException(
|
||||
"OpcUaClient session was lost before the subscription could be created.");
|
||||
@@ -1925,9 +1958,23 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
}
|
||||
|
||||
await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false);
|
||||
ThrowIfSubscribeSuperseded(subscribeGeneration);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Review m10: a subscribe that a shutdown / re-initialise overtook must not register its subscription — the
|
||||
/// teardown has already enumerated the registry, so the entry would outlive it and the next session's stall
|
||||
/// check would declare a dead subscription lost. Thrown inside the create's try, so the half-built
|
||||
/// subscription is removed like any other failed create.
|
||||
/// </summary>
|
||||
private void ThrowIfSubscribeSuperseded(long subscribeGeneration)
|
||||
{
|
||||
if (Volatile.Read(ref _lifecycleGeneration) != subscribeGeneration)
|
||||
throw new InvalidOperationException(
|
||||
"OpcUaClient session was released (shutdown or re-initialise) while the subscription was being created.");
|
||||
}
|
||||
|
||||
/// <inheritdoc />
|
||||
public async Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -2151,6 +2198,17 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
private void RecordDeviceAnswered(long generation, DateTime now)
|
||||
{
|
||||
if (Volatile.Read(ref _subscriptionLost) == 1) return;
|
||||
// Review m7: during a published outage (bad keep-alive) a reply that was in flight is evidence — the stamp
|
||||
// advances — but it does not declare the outage over; the keep-alive recovery path does that (it also
|
||||
// re-publishes the refs it published Bad), so Healthy is only written once the outage latch is clear.
|
||||
if (Volatile.Read(ref _sessionLost) == 1)
|
||||
{
|
||||
UpdateHealth(generation, h => h.State is DriverState.Healthy or DriverState.Degraded
|
||||
? h with { LastSuccessfulRead = now }
|
||||
: null);
|
||||
return;
|
||||
}
|
||||
|
||||
UpdateHealth(generation, _ => new DriverHealth(DriverState.Healthy, now, null));
|
||||
}
|
||||
|
||||
@@ -2336,11 +2394,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
MonitoredItem? eventItem = null;
|
||||
MonitoredItemNotificationEventHandler? notifHandler = null;
|
||||
|
||||
long subscribeGeneration = 0;
|
||||
await _subscriptionGate.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
try
|
||||
{
|
||||
// Re-read Session inside the critical section: a reconnect completing while we were
|
||||
// blocked on _gate has already swapped in the new session via a re-initialise.
|
||||
subscribeGeneration = Volatile.Read(ref _lifecycleGeneration);
|
||||
var session = Session
|
||||
?? throw new InvalidOperationException(
|
||||
"OpcUaClient session was lost before the alarm subscription could be created.");
|
||||
@@ -2404,6 +2464,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
eventItem.Notification += notifHandler;
|
||||
subscription.AddItem(eventItem);
|
||||
await subscription.CreateItemsAsync(cancellationToken).ConfigureAwait(false);
|
||||
ThrowIfSubscribeSuperseded(subscribeGeneration);
|
||||
|
||||
_alarmSubscriptions[id] = new RemoteAlarmSubscription(subscription, handle, eventItem, notifHandler);
|
||||
}
|
||||
@@ -2440,7 +2501,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
// Fast-fail before queuing if the driver isn't connected, but do NOT bind the ack calls
|
||||
// (or the namespace-relative ConditionId parse) to this reference — a reconnect can swap
|
||||
// Session while we wait on _gate. The session actually used is re-read inside the gate.
|
||||
_ = RequireSession();
|
||||
// Review m5: the same BadNotConnected as the in-gate check, so "no session" is one status to the caller.
|
||||
if (Session is null)
|
||||
throw new ServiceResultException(StatusCodes.BadNotConnected,
|
||||
"OpcUaClientDriver is not connected; the acknowledgement was not sent.");
|
||||
|
||||
await _gate.WaitAsync(cancellationToken).ConfigureAwait(false);
|
||||
try
|
||||
@@ -2490,7 +2554,15 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
});
|
||||
}
|
||||
|
||||
if (callRequests.Count == 0) return;
|
||||
if (callRequests.Count == 0)
|
||||
{
|
||||
// Review m3: nothing resolved to a condition node, so nothing was sent — that is not an applied ack.
|
||||
_logger.LogWarning(
|
||||
"OpcUaClientDriver '{DriverInstanceId}': AcknowledgeAsync resolved none of {Count} condition id(s) " +
|
||||
"against the live session; nothing was sent upstream.", _driverInstanceId, acknowledgements.Count);
|
||||
throw new ServiceResultException(StatusCodes.BadNodeIdUnknown,
|
||||
"None of the acknowledged condition ids resolved to an upstream condition node.");
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
@@ -2528,6 +2600,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
_logger.LogWarning(ex,
|
||||
"OpcUaClientDriver '{DriverInstanceId}': AcknowledgeAsync transport error.",
|
||||
_driverInstanceId);
|
||||
// Review m4: a timeout (not the caller cancelling) is a call with no reply, like a read or write.
|
||||
if (!cancellationToken.IsCancellationRequested) RecordCommsFault(generation, $"ack: {ex.Message}");
|
||||
throw;
|
||||
}
|
||||
catch (Exception ex)
|
||||
@@ -2538,6 +2612,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
"OpcUaClientDriver '{DriverInstanceId}': AcknowledgeAsync failed; " +
|
||||
"acknowledgements were not applied.",
|
||||
_driverInstanceId);
|
||||
RecordCommsFault(generation, $"ack: {ex.Message}");
|
||||
throw;
|
||||
}
|
||||
}
|
||||
@@ -3524,7 +3599,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
|
||||
{
|
||||
try
|
||||
{
|
||||
if (gate.Wait(TimeSpan.FromSeconds(2)))
|
||||
if (gate.Wait(TimeSpan.FromMilliseconds(500)))
|
||||
gate.Release();
|
||||
}
|
||||
catch { /* timeout or already disposed — proceed */ }
|
||||
|
||||
+1
-1
@@ -219,7 +219,7 @@
|
||||
<div class="col-md-3">
|
||||
<label class="form-label">Admin UI probe timeout (seconds)</label>
|
||||
<InputNumber @bind-Value="_form.OpcUa.ProbeTimeoutSeconds" @bind-Value:after="EmitAsync" class="form-control form-control-sm" />
|
||||
<div class="form-text">Max 60. Used by Test Connect. Default 15.</div>
|
||||
<div class="form-text">Caps Test Connect: it can only shorten the 10 s Test Connect budget, never extend it.</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
+23
@@ -100,4 +100,27 @@ public sealed class OpcUaClientBrowseSessionAttributesTests
|
||||
await Should.ThrowAsync<ArgumentException>(() =>
|
||||
session.AttributesAsync("nsu=urn:not-in-table;s=X", TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
private sealed class TrackingCredential : IDisposable
|
||||
{
|
||||
public int Disposed { get; private set; }
|
||||
|
||||
public void Dispose() => Disposed++;
|
||||
}
|
||||
|
||||
/// <summary>Review m9: the browse session disposes the user certificate it was handed, with itself.</summary>
|
||||
[Fact]
|
||||
public async Task The_session_disposes_the_credential_it_owns()
|
||||
{
|
||||
var mock = SessionReturning(NodeClass.Variable, DataTypeIds.Int32, -1, 1);
|
||||
var credential = new TrackingCredential();
|
||||
var session = new OpcUaClientBrowseSession(
|
||||
mock.Object, NamespaceMap.FromTable(new NamespaceTable()), ObjectIds.ObjectsFolder, credential);
|
||||
|
||||
credential.Disposed.ShouldBe(0, "positive control: still owned while the session is open");
|
||||
await session.DisposeAsync();
|
||||
await session.DisposeAsync();
|
||||
|
||||
credential.Disposed.ShouldBe(1);
|
||||
}
|
||||
}
|
||||
|
||||
+35
@@ -80,4 +80,39 @@ public sealed class OpcUaClientAcknowledgeFailureTests
|
||||
|
||||
await Should.ThrowAsync<ServiceResultException>(() => call);
|
||||
}
|
||||
|
||||
/// <summary>Review m3: an ack none of whose condition ids resolves sent nothing upstream — that fails, it is not "applied".</summary>
|
||||
[Fact]
|
||||
public async Task An_ack_with_no_resolvable_condition_fails_with_BadNodeIdUnknown()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-m3");
|
||||
var session = NewSession();
|
||||
drv.SetSessionForTest(session.Object);
|
||||
|
||||
var ex = await Should.ThrowAsync<ServiceResultException>(() => drv.AcknowledgeAsync(
|
||||
[new AlarmAcknowledgeRequest("nsu=urn:not-in-table;s=X", "nsu=urn:not-in-table;s=X", "ack")],
|
||||
TestContext.Current.CancellationToken));
|
||||
|
||||
ex.StatusCode.ShouldBe(StatusCodes.BadNodeIdUnknown);
|
||||
session.Verify(s => s.CallAsync(It.IsAny<RequestHeader>(), It.IsAny<CallMethodRequestCollection>(), It.IsAny<CancellationToken>()),
|
||||
Times.Never);
|
||||
}
|
||||
|
||||
/// <summary>Review m4: an ack with no reply is a comms fault like a read or write — Degraded, stamp preserved.</summary>
|
||||
[Fact]
|
||||
public async Task An_ack_transport_failure_degrades_and_preserves_the_stamp()
|
||||
{
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-m4");
|
||||
var stamp = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc);
|
||||
drv.SetHealthForTest(new DriverHealth(DriverState.Healthy, stamp, null));
|
||||
var session = NewSession();
|
||||
session.Setup(s => s.CallAsync(It.IsAny<RequestHeader>(), It.IsAny<CallMethodRequestCollection>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new ServiceResultException(StatusCodes.BadConnectionClosed));
|
||||
drv.SetSessionForTest(session.Object);
|
||||
|
||||
await Should.ThrowAsync<ServiceResultException>(() => drv.AcknowledgeAsync(Ack(), TestContext.Current.CancellationToken));
|
||||
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
drv.GetHealth().LastSuccessfulRead.ShouldBe(stamp);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -51,15 +51,17 @@ public sealed class OpcUaClientAlarmTests
|
||||
await drv.UnsubscribeAlarmsAsync(new FakeAlarmHandle(), TestContext.Current.CancellationToken);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that AcknowledgeAsync without initialize throws InvalidOperationException.</summary>
|
||||
/// <summary>Verifies that AcknowledgeAsync without initialize throws ServiceResultException(BadNotConnected).</summary>
|
||||
[Fact]
|
||||
public async Task AcknowledgeAsync_without_initialize_throws_InvalidOperationException()
|
||||
public async Task AcknowledgeAsync_without_initialize_throws_BadNotConnected()
|
||||
{
|
||||
// Review m5: the same status as a session lost while the ack queued (one "no session" answer).
|
||||
using var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions(), "opcua-ack-uninit");
|
||||
await Should.ThrowAsync<InvalidOperationException>(async () =>
|
||||
var ex = await Should.ThrowAsync<Opc.Ua.ServiceResultException>(async () =>
|
||||
await drv.AcknowledgeAsync(
|
||||
[new AlarmAcknowledgeRequest("ns=2;s=Src", "ns=2;s=Cond", "operator ack")],
|
||||
TestContext.Current.CancellationToken));
|
||||
ex.StatusCode.ShouldBe(Opc.Ua.StatusCodes.BadNotConnected);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that AcknowledgeAsync with empty batch is noop even without init.</summary>
|
||||
|
||||
+43
@@ -117,4 +117,47 @@ public sealed class OpcUaClientWriteHealthTests
|
||||
h.LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(Stale);
|
||||
h.LastError.ShouldBeNull();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Review m8: a write that TIMED OUT (an OperationCanceledException the caller did not cause) is a call with no
|
||||
/// reply — Degraded, stamp preserved. The genuine positive control for the caller-cancelled test above (same
|
||||
/// catch branch, the caller's token is the only difference).
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_write_that_timed_out_without_caller_cancellation_degrades()
|
||||
{
|
||||
var (drv, session) = NewConnectedDriver(new DriverHealth(DriverState.Healthy, Stale, null));
|
||||
using var _ = drv;
|
||||
session.Setup(s => s.WriteAsync(It.IsAny<RequestHeader>(), It.IsAny<WriteValueCollection>(), It.IsAny<CancellationToken>()))
|
||||
.ThrowsAsync(new OperationCanceledException("SDK request timeout"));
|
||||
|
||||
var results = await drv.WriteAsync(Write(), TestContext.Current.CancellationToken);
|
||||
|
||||
results.Single().StatusCode.ShouldBe(0x800A0000u); // BadTimeout
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
drv.GetHealth().LastSuccessfulRead.ShouldBe(Stale);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Review m7: a reply that was in flight across a bad keep-alive advances the stamp (it is evidence) but does not
|
||||
/// declare the published outage over — health stays Degraded until the keep-alive recovers.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task A_reply_during_a_published_outage_advances_the_stamp_but_keeps_degraded()
|
||||
{
|
||||
var (drv, session) = NewConnectedDriver(new DriverHealth(DriverState.Healthy, Stale, null));
|
||||
using var _ = drv;
|
||||
drv.RaiseKeepAliveForTest(StatusCodes.BadNoCommunication);
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Degraded, "positive control: the outage is published");
|
||||
session.Setup(s => s.WriteAsync(It.IsAny<RequestHeader>(), It.IsAny<WriteValueCollection>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new WriteResponse { ResponseHeader = new ResponseHeader(), Results = [StatusCodes.Good], DiagnosticInfos = [] });
|
||||
|
||||
await drv.WriteAsync(Write(), TestContext.Current.CancellationToken);
|
||||
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Degraded);
|
||||
drv.GetHealth().LastSuccessfulRead.ShouldNotBeNull().ShouldBeGreaterThan(Stale);
|
||||
|
||||
drv.RaiseKeepAliveForTest(StatusCodes.Good);
|
||||
drv.GetHealth().State.ShouldBe(DriverState.Healthy, "the keep-alive recovery is what ends the outage");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user