Files
UA-.NETStandard/samples/ConsoleReferenceServer/UAServer.cs
Marc Schier e05a820c91 Removal of the exposed diagnostics and node locks (#4188)
## What this is

Design analysis for `Opc.Ua.Server`, the two rounds of design
exploration behind it, and the first two implementation chunks it
recommends.

Draft: the analyses are ready to read, but the *conclusions* deserve
discussion before the larger phases proceed. Later chunks stack onto
this PR as they land.

| File | Kind | Lines |
|---|---|---:|
| `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin
surface | 488 |
| `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime
cluster | 576 |
| `plans/ServerDesignExploration.md` | Design round 2 — delivery
strategy | 1,394 |
| `plans/ServerDesignExplorationAlt.md` | Design round 1 — four
interface designs | 2,119 |
| `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards
attributes | |
| `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value
accessors take the lock | |
| `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** —
6 external locks removed | |
| `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** —
concurrency safety net | 449 |

`docs/` is untouched — it holds shipped feature documentation, and none
of this is shipped API.

## The analyses

Counted mechanically over `src`, `samples` and `tests`; **every figure
is reproducible** from commands printed in each document.

**Node manager surface.** `AsyncCustomNodeManager` declares **93
`virtual`/`abstract` members** (80 distinct names). Across the
repository: **112 override sites, 25 distinct names ever overridden, 55
of 80 never**. Six carry nearly all real use. For scale,
`ReferenceNodeManager` is 6,373 lines and overrides **8**.

**Server runtime.** Seven findings, **two of which recommend no
change**. Findings 1–5 are filed as #4183–#4187. The headline is an
`object`-typed sync root published on three public interfaces and taken
in **88 `lock` statements across 7 files**, including a shipped sample.

**What is deliberately not flagged.** `IStandardServer` — 9 members over
5,575 lines, 16 of 37 virtuals used — is identified as the reference
shape to preserve. `IApplicationInstance` and the already-`[Obsolete]`
`Stop()` are examined and left alone.

## The design exploration

**Round 1** explored four alternative interfaces in parallel under the
current back-compat constraint. Its value is the convergence: four
designers who never saw each other's work independently arrived at a 6–9
member `IServerContext` replacing the 57-member locator, removal of the
`object` node-manager handle, elimination of the `Processed` protocol,
and `IHistorianProvider` as the pattern to copy.

**Round 2** re-runs it with back-compat lifted and asks how to deliver:
full rewrite versus side-by-side fork. **Both option designers
independently declined to recommend their own assignment.** Three
structural facts were verified:

1. **The fork line already sits at `IAsyncNodeManager`** — old and new
node managers can coexist with zero runtime changes, so a fork
duplicates ~10% of the assembly, not all of it.
2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest
auto-fix ceiling for the authoring surface is **~10%**, against ~70% for
the shipped analyzers.
3. **A second lock leak exists** — `NodeState` takes `lock (this)` under
a `CA2002` suppression whose own comment documents the contract.

Recommendation: **neither pure option**, but a staged path reaching the
rewrite end state incrementally, gated on migrating the five
framework-internal node managers that today make it impossible for the
old base to ever freeze.

## Implementation: NodeState now guards its own attributes

This removes the prose-only contract in which `NodeState` took
`lock(this)` and every external caller was required to take
`lock(source)` on the node instance. The code already carried a TODO
describing exactly this fix and noting it required updating every
external site.

- The node self-guards rather than relying on callers.
`BaseVariableState` gains a **fully private** `System.Threading.Lock`
covering its value fields; `NodeState`'s other collections keep their
existing private locks.
- All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions
are gone.
- `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the
lock too — without that, a direct property write raced an attribute read
once the external lock was gone.
- **6 of 9** external sites removed. Five were already redundant
(`GetReferences`, `GetChildren`, `ReferenceExists` and
`SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state
via `GetChildren`); the sixth was an attribute read now covered by the
node's own lock.
- Attribute read/write deliberately take **no** node-wide lock: both
dispatch to caller-supplied handlers (`OnReadValue` and siblings) that
may take locks of their own, and holding a lock on the node across one
gives the node no fixed place in the lock order.
- **3 deliberately retained and annotated in code**: two `CreateBrowser`
calls, which are not internally synchronized (tracked in #4216), and one
in `CustomNodeManager`, whose class is slated for deletion so
refactoring its lock would be wasted work.

### No lock is exposed anywhere, to anyone

An earlier revision put the attribute lock on `NodeState` as `private
protected`, which still published a lock-sharing contract on the
inheritance surface. Review pushed back and it is gone:
`BaseVariableState` was the only user, so the lock moved there as a
plain `private readonly Lock`. There is no member for a derived type to
call, in this assembly or any other.

`BaseVariableValue` was the last public leak — it handed out the lock
guarding its value through a `public Lock` property, so any caller could
hold it across a call back into the stack. That property is removed:

- Derived value classes (what the source generator emits for every
structure variable) synchronize through a `protected EnterLock()` /
`ExitLock()` pair. Callback helpers were the first choice, but the
generated bodies pass `ref Variant` / `ref StatusCode` / `ref
DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The
lock *object* stays private either way.
- `NodeStateTemplates` emits the new form.
- `ServerInternalData`, the one cross-assembly caller, was reading the
lock back off a value it had itself constructed with that lock. It now
takes `m_diagnosticsLock` directly.

`NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a
subclass extending `Next()` legitimately needs its enumeration state
atomic with the base browser's, so the fix needs a base-owned
acquisition rather than a deletion.

### Migration coverage for the removed lock members

None of the removed lock members get an `[Obsolete]` shim, and the
migration guide now says why: a shim would have to be `object`-typed to
keep old call sites compiling, and on .NET 9+ `lock (o)` on a
statically-`object` reference is `Monitor.Enter`, which does **not**
interlock with `Lock.Enter()`. It would compile, run, and silently
synchronize nothing — strictly worse than a compile error. Re-adding a
member to a downstream-implemented interface is also a break in its own
right on `net48` / `netstandard2.0`, where there are no default
interface methods.

Instead, every removed member has a named replacement on the migration
path:

| Rule | Covers | Replacement it names |
|---|---|---|
| `UA0024` | `IServerInternal` / `ISession` / `ISubscription`
`DiagnosticsLock`, `DiagnosticsWriteLock` |
`UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`,
`ReadServerDiagnostics` / `ReadDiagnostics` |
| `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller
owns; the node guards its own state |
| `UA0026` | `BaseVariableValue.Lock` | a lock passed to the
`BaseVariableValue` constructor |

Each rule fires on sources still written against 1.5.378, where the
compiler only says a member is missing. `docs/MigrationGuide.md` gains a
section per removal with before/after.

### The tests earned their keep

The four concurrency tests were landed **first**, against the unmodified
code, because removing a synchronization contract cannot be verified by
the compiler and the existing `NodeStateHandlerConcurrencyTests` covers
a different concern (racing a handler assignment against a write, not
reader/writer exclusion). They now run with **no external locking**,
which is what demonstrates the node guards itself.

They caught two real problems, and one of them was mine:

- The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a
genuine race.
- One test asserted an invariant the design does not offer. Two property
setters are two lock acquisitions, so that pair was **never** atomic; it
only passed originally because the test held a lock across both. It now
uses a single attribute write carrying a whole `DataValue`, which
genuinely is atomic and is the path the server uses.

A third issue was found and fixed before it shipped: the reader loops
gated on `!writer.IsCompleted`, so a reader scheduled after the writer
finished performed zero reads — failing two runs in three on `net48`.
Converted to `do…while`; now five consecutive green runs.

**Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the
concurrency category green 5/5 consecutive on `net48`;
`Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205
passed, including `NodeManagerLifecycleTests` and
`AsyncCustomNodeManagerTests` which exercise the changed paths.

## Two corrections to my own published figures

Both were found while starting implementation, and both are recorded on
#4183.

**`ForceDiagnosticsScan()` was overstated.** The analysis claimed the
`DiagnosticsWriteLock` getter *"triggers a full address-space
diagnostics rescan"*. It is in fact a single flag assignment
(`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the
periodic timer. The defect stands — a side-effecting property getter
with a racy write on a synchronisation path — but its cost was
misstated.

**The contract was 15 sites, not 19.** A naive search matched ten
comment and `#pragma` lines that *mention* `lock(source)` while
explaining it. The documents now carry a counting command that excludes
comment matches.

A third finding expanded scope the other way: the `object`-typed lock
leak is **repo-wide**, with ten such members across `Opc.Ua.Core`,
`Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this`
and `ApplicationConfiguration.PropertiesLock => m_properties`, the same
defect class as `Subscription.DiagnosticsLock`.

## Caveats stated in the documents

- **Nothing in the designs is implemented or benchmarked.** Every
performance claim across all seven designs is a design intention, not a
measurement.
- The larger phases are a **major undertaking against a live plugin
API**; the documents say so rather than letting the neatness of the
proposals imply otherwise.
- The recommended sequencing puts low-risk, self-contained steps first
and notes they are **worth doing regardless** of which strategy is
eventually chosen.
- Round 2 notes the two option designers' cost estimates are **not
independent of their assignments**.

## Review guidance

The two analyses make factual claims checkable by running the included
commands. The design explorations are exploratory and need not be agreed
with to merge. The code change is small, test-backed, and independent of
which delivery strategy is eventually chosen.

Reasonable outcomes include merging as-is, merging only the analyses and
the lock fix, or disagreeing with the recommendation while keeping the
evidence.

---

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041
Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
2026-08-10 06:47:50 +02:00

431 lines
16 KiB
C#

/* ========================================================================
* Copyright (c) 2005-2025 The OPC Foundation, Inc. All rights reserved.
*
* OPC Foundation MIT License 1.00
*
* Permission is hereby granted, free of charge, to any person
* obtaining a copy of this software and associated documentation
* files (the "Software"), to deal in the Software without
* restriction, including without limitation the rights to use,
* copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following
* conditions:
*
* The above copyright notice and this permission notice shall be
* included in all copies or substantial portions of the Software.
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
* OTHER DEALINGS IN THE SOFTWARE.
*
* The complete license agreement can be found here:
* http://opcfoundation.org/License/MIT/1.00/
* ======================================================================*/
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Opc.Ua;
using Opc.Ua.Configuration;
#if NET10_0_OR_GREATER
using Opc.Ua.Pcap.Capture;
#endif
using Opc.Ua.Security.Certificates;
using Opc.Ua.Server;
namespace Quickstarts
{
public class UAServer<T> where T : StandardServer
{
public IApplicationInstance Application { get; private set; } = null!;
public ApplicationConfiguration Configuration => Application.ApplicationConfiguration!;
public bool AutoAccept { get; set; }
public char[]? Password { get; set; }
public ExitCode ExitCode { get; private set; }
public T? Server { get; private set; }
/// <summary>
/// Ctor of the server.
/// </summary>
/// <param name="telemetry">The telemetry context.</param>
public UAServer(ITelemetryContext telemetry, Func<ITelemetryContext, T> factory)
{
m_factory = factory;
m_telemetry = telemetry;
m_logger = telemetry.CreateLogger<UAServer<T>>();
}
/// <summary>
/// Load the application configuration.
/// </summary>
/// <exception cref="ErrorExitException"></exception>
public async Task LoadAsync(string applicationName, string configSectionName)
{
try
{
ExitCode = ExitCode.ErrorNotStarted;
ApplicationInstance.MessageDlg = new ApplicationMessageDlg();
var passwordProvider = new CertificatePasswordProvider(Password);
Application = new ApplicationInstance(m_telemetry)
{
ApplicationName = applicationName,
ApplicationType = ApplicationType.Server,
ConfigSectionName = configSectionName,
CertificatePasswordProvider = passwordProvider
};
// load the application configuration.
await Application.LoadApplicationConfigurationAsync(false).ConfigureAwait(false);
}
catch (Exception ex)
{
throw new ErrorExitException(ex.Message, ExitCode);
}
}
/// <summary>
/// Load the application configuration.
/// </summary>
/// <exception cref="ErrorExitException"></exception>
public async Task CheckCertificateAsync(bool renewCertificate)
{
try
{
ApplicationConfiguration config = Application.ApplicationConfiguration!;
if (renewCertificate)
{
await Application.DeleteApplicationInstanceCertificateAsync().ConfigureAwait(false);
}
// check the application certificate.
bool haveAppCertificate = await Application
.CheckApplicationInstanceCertificatesAsync(false)
.ConfigureAwait(false);
if (!haveAppCertificate)
{
throw new ErrorExitException("Application instance certificate invalid!");
}
if (!config.SecurityConfiguration.AutoAcceptUntrustedCertificates)
{
config.CertificateManager.AcceptError = AcceptCertificate;
}
}
catch (Exception ex)
{
throw new ErrorExitException(ex.Message, ExitCode);
}
}
/// <summary>
/// Create server instance and add node managers.
/// </summary>
/// <exception cref="ErrorExitException"></exception>
public void Create(ArrayOf<INodeManagerFactory> nodeManagerFactories)
{
try
{
// create the server.
Server = m_factory(m_telemetry);
foreach (INodeManagerFactory factory in nodeManagerFactories)
{
Server.AddNodeManager(factory);
}
}
catch (Exception ex)
{
throw new ErrorExitException(ex.Message, ExitCode);
}
}
/// <summary>
/// Start the server.
/// </summary>
/// <exception cref="ErrorExitException"></exception>
public async Task StartAsync(CancellationToken ct = default)
{
try
{
// create the server.
Server ??= m_factory(m_telemetry);
#if NET10_0_OR_GREATER
// Opt-in diagnostics: when OPCUA_PCAP_FILE / OPCUA_KEYLOGFILE are
// set, install server-side pcap capture into the transport
// bindings before the listeners open. Mirrors the client env-var
// path and honours the same variables; a complete no-op (nothing
// installed, no files written) when the variables are unset.
// The pcap capture package targets net8.0+ only.
m_pcapCapture = await PcapServerCapture.TryStartFromEnvironmentAsync(
Server!.TransportBindings,
m_telemetry.LoggerFactory,
ct).ConfigureAwait(false);
#endif
// start the server
await Application.StartAsync(Server, ct).ConfigureAwait(false);
// save state
ExitCode = ExitCode.ErrorRunning;
// print endpoint info
foreach (string? endpoint in Application.Server!
.GetEndpoints()
.ConvertAll(e => e.EndpointUrl)
.ToList()
.Distinct())
{
Console.WriteLine(endpoint);
}
// start the status thread
m_status = Task.Run(StatusThreadAsync, default);
// print notification on session events
Server.CurrentInstance.SessionManager.SessionActivated += EventStatus;
Server.CurrentInstance.SessionManager.SessionClosing += EventStatus;
Server.CurrentInstance.SessionManager.SessionCreated += EventStatus;
}
catch (Exception ex)
{
throw new ErrorExitException(ex.Message, ExitCode);
}
}
/// <summary>
/// Stops the server.
/// </summary>
/// <exception cref="ErrorExitException"></exception>
public async Task StopAsync(CancellationToken ct = default)
{
try
{
if (Server != null)
{
using T server = Server;
// Stop status thread which monitores the server property.
Server = null;
await m_status.ConfigureAwait(false);
// Stop server and dispose
await server.StopAsync(ct).ConfigureAwait(false);
}
#if NET10_0_OR_GREATER
// Stop any env-var-driven pcap capture installed at start.
if (m_pcapCapture != null)
{
await m_pcapCapture.DisposeAsync().ConfigureAwait(false);
m_pcapCapture = null;
}
#endif
ExitCode = ExitCode.Ok;
}
catch (Exception ex)
{
throw new ErrorExitException(ex.Message, ExitCode.ErrorStopping);
}
}
/// <summary>
/// Per-error accept callback used when AutoAcceptUntrustedCertificates is false.
/// </summary>
private bool AcceptCertificate(Certificate certificate, ServiceResult error)
{
if (error.StatusCode == StatusCodes.BadCertificateUntrusted && AutoAccept)
{
m_logger.AcceptedCertificate(
certificate.Subject,
certificate.Thumbprint);
return true;
}
m_logger.RejectedCertificate(
error,
certificate.Subject,
certificate.Thumbprint);
return false;
}
/// <summary>
/// Update the session status.
/// </summary>
private void EventStatus(ISession session, SessionEventReason reason)
{
m_lastEventTime = DateTime.UtcNow;
LogSessionStatusFull(session, reason.ToString());
}
/// <summary>
/// Output the status of a connected session.
/// </summary>
private void LogSessionStatusLastContact(ISession session, string reason)
{
if (!m_logger.IsEnabled(LogLevel.Information))
{
return;
}
// Snapshot the fields under the session's lock, then log outside it.
(string? name, DateTime lastContact) = session.ReadDiagnostics(
diagnostics => (
diagnostics.SessionName,
(DateTime)diagnostics.ClientLastContactTime));
m_logger.SessionStatusLastContact(reason, name, lastContact.ToLocalTime());
}
/// <summary>
/// Output the status of a connected session.
/// </summary>
private void LogSessionStatusFull(ISession session, string reason)
{
if (!m_logger.IsEnabled(LogLevel.Information))
{
return;
}
(string? name, DateTime lastContact) = session.ReadDiagnostics(
diagnostics => (
diagnostics.SessionName,
(DateTime)diagnostics.ClientLastContactTime));
m_logger.SessionStatusFull(
reason,
name,
lastContact.ToLocalTime(),
session.Identity?.DisplayName ?? "Anonymous",
session.Id);
}
/// <summary>
/// Status thread, prints connection status every 10 seconds.
/// </summary>
private async Task StatusThreadAsync()
{
while (Server != null)
{
if (DateTime.UtcNow - m_lastEventTime > TimeSpan.FromMilliseconds(10000))
{
IList<ISession> sessions = Server.CurrentInstance.SessionManager.GetSessions();
for (int ii = 0; ii < sessions.Count; ii++)
{
ISession session = sessions[ii];
LogSessionStatusLastContact(session, "-Status-");
}
m_lastEventTime = DateTime.UtcNow;
}
await Task.Delay(1000).ConfigureAwait(false);
}
}
/// <summary>
/// A dialog which asks for user input.
/// </summary>
public class ApplicationMessageDlg : IApplicationMessageDlg
{
private readonly TextWriter m_output;
private string m_message = string.Empty;
private bool m_ask;
public ApplicationMessageDlg(TextWriter? output = null)
{
m_output = output ?? Console.Out;
}
public override void Message(string text, bool ask)
{
m_message = text;
m_ask = ask;
}
public override async Task<bool> ShowAsync()
{
if (m_ask)
{
var message = new StringBuilder(m_message);
message.Append(" (y/n, default y): ");
m_output.Write(message.ToString());
try
{
ConsoleKeyInfo result = Console.ReadKey();
m_output.WriteLine();
return await Task.FromResult(result.KeyChar is 'y' or 'Y' or '\r')
.ConfigureAwait(false);
}
catch
{
// intentionally fall through
}
}
else
{
m_output.WriteLine(m_message);
}
return await Task.FromResult(true).ConfigureAwait(false);
}
}
private readonly Func<ITelemetryContext, T> m_factory;
private readonly ITelemetryContext m_telemetry;
private readonly ILogger m_logger;
private Task m_status = Task.CompletedTask;
private DateTime m_lastEventTime;
#if NET10_0_OR_GREATER
private IAsyncDisposable? m_pcapCapture;
#endif
}
internal static partial class UAServerLog
{
[LoggerMessage(EventId = ConsoleReferenceServerEventIds.UAServer + 0, Level = LogLevel.Information,
Message = "Accepted Certificate: [{Subject}] [{Thumbprint}]")]
public static partial void AcceptedCertificate(this ILogger logger, string subject, string thumbprint);
[LoggerMessage(EventId = ConsoleReferenceServerEventIds.UAServer + 1, Level = LogLevel.Information,
Message = "Rejected Certificate: {Error} [{Subject}] [{Thumbprint}]")]
public static partial void RejectedCertificate(
this ILogger logger,
ServiceResult error,
string subject,
string thumbprint);
[LoggerMessage(EventId = ConsoleReferenceServerEventIds.UAServer + 2, Level = LogLevel.Information,
Message = "{Reason,9}:{Session,20}:Last Event:{LastContactTime:HH:mm:ss}")]
public static partial void SessionStatusLastContact(
this ILogger logger,
string reason,
string? session,
DateTime lastContactTime);
[LoggerMessage(EventId = ConsoleReferenceServerEventIds.UAServer + 3, Level = LogLevel.Information,
Message = "{Reason,9}:{Session,20}:Last Event:{LastContactTime:HH:mm:ss}:{UserIdentity,20}:{SessionId}")]
public static partial void SessionStatusFull(
this ILogger logger,
string reason,
string? session,
DateTime lastContactTime,
string userIdentity,
NodeId sessionId);
}
}