fix: preserve SDK 2 driver value and credential lifetimes

This commit is contained in:
Joseph Doherty
2026-11-10 10:17:03 -05:00
parent f9a006dab4
commit 4400127810
5 changed files with 73 additions and 48 deletions
@@ -17,7 +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;
private readonly IAsyncDisposable? _ownedApplication;
/// <summary>
/// Construct a browse session bound to an already-connected <paramref name="session"/>.
@@ -26,10 +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>
/// <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)
/// <param name="ownedApplication">The SDK application whose certificate manager this session owns.</param>
internal OpcUaClientBrowseSession(ISession session, NamespaceMap nsMap, NodeId rootNodeId, IAsyncDisposable? ownedApplication = null)
{
_ownedCredential = ownedCredential;
_ownedApplication = ownedApplication;
_session = session;
_nsMap = nsMap;
_rootNodeId = rootNodeId;
@@ -177,11 +177,12 @@ internal sealed class OpcUaClientBrowseSession : IBrowseSession
// Best-effort — mirrors OpcUaClientDriver's pagination loop.
try
{
using var release = new CancellationTokenSource(TimeSpan.FromSeconds(5));
await _session.BrowseNextAsync(
requestHeader: null,
releaseContinuationPoints: true,
continuationPoints: [cp],
ct: CancellationToken.None).ConfigureAwait(false);
ct: release.Token).AsTask().WaitAsync(release.Token).ConfigureAwait(false);
}
catch { /* best-effort — server may have already cleaned up */ }
throw;
@@ -246,7 +247,9 @@ internal sealed class OpcUaClientBrowseSession : IBrowseSession
try { _gate.Dispose(); }
catch { /* best-effort */ }
try { _ownedCredential?.Dispose(); }
catch { /* best-effort */ }
if (_ownedApplication is not null)
{
try { await _ownedApplication.DisposeAsync().ConfigureAwait(false); } catch { /* best-effort */ }
}
}
}
@@ -69,9 +69,10 @@ 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, ownedCertificate) = await BuildBrowseCredentialAsync(opts, _secretResolver, cancellationToken)
var identity = await BuildBrowseUserIdentityAsync(opts, _secretResolver, cancellationToken)
.ConfigureAwait(false);
ApplicationInstance? ownedApplication = null;
try
{
if (opts.AutoAcceptCertificates)
@@ -81,7 +82,9 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
"driver's PKI store instead.",
endpoint);
var appConfig = await BuildBrowseAppConfigurationAsync(cancellationToken).ConfigureAwait(false);
var app = await BuildBrowseAppConfigurationAsync(cancellationToken).ConfigureAwait(false);
ownedApplication = app;
var appConfig = app.ApplicationConfiguration!;
var perEndpointBudget = TimeSpan.FromSeconds(
Math.Clamp(opts.PerEndpointConnectTimeout.TotalSeconds, 5, 30));
@@ -106,6 +109,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
try
{
connectCts.Token.ThrowIfCancellationRequested();
var nsMap = NamespaceMap.FromSession(session);
var rootNodeId = string.IsNullOrEmpty(opts.BrowseRoot)
? ObjectIds.ObjectsFolder
@@ -115,7 +119,9 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
"AdminUI OPC UA browse session opened against {Endpoint} (policy {Policy}, mode {Mode})",
endpoint, opts.SecurityPolicy, opts.SecurityMode);
return new OpcUaClientBrowseSession(session, nsMap, rootNodeId, ownedCertificate);
var browse = new OpcUaClientBrowseSession(session, nsMap, rootNodeId, ownedApplication);
ownedApplication = null;
return browse;
}
catch
{
@@ -126,9 +132,8 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
}
catch
{
// 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();
// The SDK application owns its certificate manager until the browse session takes it.
if (ownedApplication is not null) await ownedApplication.DisposeAsync().ConfigureAwait(false);
throw;
}
}
@@ -139,7 +144,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
/// trust decisions don't leak into the deployed driver's cert store.
/// </summary>
/// <param name="ct">Cancellation token for the configuration build.</param>
private static async Task<ApplicationConfiguration> BuildBrowseAppConfigurationAsync(CancellationToken ct)
private static async Task<ApplicationInstance> BuildBrowseAppConfigurationAsync(CancellationToken ct)
{
// The ITelemetryContext overload is the non-obsolete ctor; an empty builder keeps SDK
// internals off any logging provider.
@@ -153,7 +158,7 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
"OtOpcUa", "adminui-browse-pki");
var config = new ApplicationConfiguration
var config = new ApplicationConfiguration(BrowseTelemetry.Context)
{
ApplicationName = "OtOpcUa AdminUI Browse",
ApplicationType = ApplicationType.Client,
@@ -187,11 +192,19 @@ public sealed class OpcUaClientDriverBrowser : IDriverBrowser
ClientConfiguration = new ClientConfiguration { DefaultSessionTimeout = 60_000 },
DisableHiResClock = true,
};
await config.ValidateAsync(ApplicationType.Client, ct).ConfigureAwait(false);
app.ApplicationConfiguration = config;
await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct)
.ConfigureAwait(false);
return config;
try
{
await config.ValidateAsync(ApplicationType.Client, ct).ConfigureAwait(false);
await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct)
.ConfigureAwait(false);
return app;
}
catch
{
await app.DisposeAsync().ConfigureAwait(false);
throw;
}
}
/// <summary>
@@ -203,28 +216,11 @@ 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) =>
(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)
OpcUaClientDriverOptions opts, ISecretResolver? secretResolver, CancellationToken ct)
{
ct.ThrowIfCancellationRequested();
var resolved = await OpcUaUserIdentityFactory.ResolveSecretRefsAsync(opts, secretResolver, ct).ConfigureAwait(false);
if (resolved.AuthType != OpcUaAuthType.Certificate)
{
return (OpcUaUserIdentityFactory.Build(resolved), null);
}
return (OpcUaUserIdentityFactory.FromCertificate(resolved), null);
return OpcUaUserIdentityFactory.Build(resolved);
}
/// <summary>Select the endpoint matching the requested SecurityPolicy + SecurityMode pair.</summary>
@@ -19,7 +19,8 @@ public static class UpstreamValueMapping
DateTimeUtc[] times => times.Select(t => t.ToDateTime()).ToArray(),
Uuid[] uuids => uuids.Select(u => (Guid)u).ToArray(),
Variant[] variants => variants.Select(ToClr).ToArray(),
object?[] objects => objects.Select(Detach).ToArray(),
byte[][] bytes => bytes.Select(b => b?.ToArray()).ToArray(),
object?[] objects when objects.GetType() == typeof(object[]) => objects.Select(Detach).ToArray(),
Array array => array.Clone(),
_ => value,
};
@@ -280,6 +280,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
private Func<Opc.Ua.Security.Certificates.Certificate, ServiceResult, bool>? _certValidationHandler;
/// <summary>The <see cref="ICertificateManager"/> that owns <see cref="_certValidationHandler"/>.</summary>
private ICertificateManager? _certValidatorRef;
private ApplicationInstance? _ownedApplication;
/// <summary>
/// 1 while the CURRENT session's keep-alive is bad (an outage is in progress), else 0. The
/// 0→1 edge publishes the outage once (Bad per subscribed ref + a quality-only alarm annotation); the 1→0
@@ -565,6 +566,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
string? connectedUrl = null;
foreach (var url in candidates)
{
cancellationToken.ThrowIfCancellationRequested();
try
{
session = OpenSessionForTest is { } open
@@ -572,9 +574,11 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
: await OpenSessionOnEndpointAsync(
appConfig, url, _options.SecurityPolicy, _options.SecurityMode,
identity, cancellationToken).ConfigureAwait(false);
cancellationToken.ThrowIfCancellationRequested();
connectedUrl = url;
break;
}
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) { throw; }
catch (Exception ex)
{
attemptErrors.Add($"{url} -> {ex.GetType().Name}: {ex.Message}");
@@ -622,6 +626,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
DetachCertValidation(_certValidatorRef, _certValidationHandler);
_certValidationHandler = certValidation.Handler;
_certValidatorRef = certValidation.Validator;
_ownedApplication = certValidation.Application;
installed = true;
}
}
@@ -642,7 +647,12 @@ 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 (!installed)
{
DetachCertValidation(certValidation.Validator, certValidation.Handler);
if (certValidation.Application is { } application)
await application.DisposeAsync().ConfigureAwait(false);
}
if (Volatile.Read(ref _lifecycleGeneration) == generation)
{
if (installed)
@@ -659,6 +669,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
{
public Func<Opc.Ua.Security.Certificates.Certificate, ServiceResult, bool>? Handler { get; set; }
public ICertificateManager? Validator { get; set; }
public ApplicationInstance? Application { get; set; }
}
private static void DetachCertValidation(ICertificateManager? validator, Func<Opc.Ua.Security.Certificates.Certificate, ServiceResult, bool>? handler)
@@ -735,11 +746,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
ApplicationType = ApplicationType.Client,
};
certValidation.Application = app;
var pkiRoot = Path.Combine(
Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData),
"OtOpcUa", "pki");
var config = new ApplicationConfiguration
var config = new ApplicationConfiguration(_sdkTelemetry)
{
ApplicationName = _options.SessionName,
ApplicationType = ApplicationType.Client,
@@ -782,6 +794,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
DisableHiResClock = true,
};
app.ApplicationConfiguration = config;
await config.ValidateAsync(ApplicationType.Client, ct).ConfigureAwait(false);
// AutoAccept=true is a dev-only escape hatch. Emit a prominent warning so a
@@ -821,7 +834,6 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
}
// Ensure an application certificate exists. The SDK auto-generates one if missing.
app.ApplicationConfiguration = config;
await app.CheckApplicationInstanceCertificatesAsync(silent: true, lifeTimeInMonths: null, ct)
.ConfigureAwait(false);
@@ -1181,10 +1193,13 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
// Take the session reference under _probeLock and clear Session to null so any concurrent caller that
// checks inside _gate sees null immediately after release begins.
ISession? sessionToClose;
ApplicationInstance? applicationToDispose;
lock (_probeLock)
{
if (Volatile.Read(ref _lifecycleGeneration) != generation) return;
sessionToClose = Session;
applicationToDispose = _ownedApplication;
_ownedApplication = null;
if (_keepAliveHandler is not null && sessionToClose is not null)
{
try { sessionToClose.KeepAlive -= _keepAliveHandler; } catch { }
@@ -1209,6 +1224,10 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
}
catch { /* best-effort */ }
try { sessionToClose?.Dispose(); } catch { }
if (applicationToDispose is not null)
{
try { await applicationToDispose.DisposeAsync().ConfigureAwait(false); } catch { }
}
Interlocked.Exchange(ref _sessionLost, 0);
Interlocked.Exchange(ref _subscriptionLost, 0);
Interlocked.Exchange(ref _publishingStoppedSinceTicks, 0);
@@ -1732,11 +1751,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
// the release fails silently and the cursor times out server-side anyway.
try
{
using var release = new CancellationTokenSource(TimeSpan.FromSeconds(5));
await session.BrowseNextAsync(
requestHeader: null,
releaseContinuationPoints: true,
continuationPoints: [continuationPoint],
ct: CancellationToken.None).ConfigureAwait(false);
ct: release.Token).AsTask().WaitAsync(release.Token).ConfigureAwait(false);
}
catch { /* best-effort — server may have already cleaned up */ }
throw;
@@ -2630,7 +2650,7 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
ObjectId = conditionId,
MethodId = MethodIds.AcknowledgeableConditionType_Acknowledge,
InputArguments = [
new Variant(eventId), // Part 9 §5.7.3: the EventId being acknowledged (empty only when none seen)
Variant.From(eventId.ToByteString()), // Part 9 §5.7.3: the EventId being acknowledged (empty only when none seen)
new Variant(new LocalizedText(ack.Comment ?? string.Empty)),
],
});
@@ -3702,8 +3722,8 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
/// hook synchronously (no async work on this hot path), then fires the cert-validation
/// handler detach. The async session-close is intentionally skipped — it requires a
/// live session + network round-trip and is unsafe to block-on from a potentially
/// single-threaded context (OPC UA stack thread). The session will be cleaned up by
/// the SDK's own finalizer on GC (no sync-over-async).
/// single-threaded context (OPC UA stack thread). Local session and certificate-manager
/// resources are disposed synchronously.
/// </summary>
public void Dispose()
{
@@ -3721,7 +3741,12 @@ public sealed class OpcUaClientDriver : IDriver, ITagDiscovery, IReadable, IWrit
try { Session.KeepAlive -= _keepAliveHandler; } catch { }
}
_keepAliveHandler = null;
var session = Session;
Session = null;
if (session is not null) DetachDiagnostics(session);
try { session?.Dispose(); } catch { }
try { (_ownedApplication?.ApplicationConfiguration?.CertificateManager as IDisposable)?.Dispose(); } catch { }
_ownedApplication = null;
}
// Detach the cert-validation handler registered during InitializeAsync so the
@@ -130,7 +130,7 @@ public sealed class OpcUaClientDriverProbe : IDriverProbe
/// is plain-text and does not require a client certificate.
/// </summary>
private static ApplicationConfiguration BuildMinimalAppConfig() =>
new()
new(SdkTelemetry.Context)
{
ApplicationName = "OtOpcUa.Probe",
ApplicationType = ApplicationType.Client,