Files
UA-.NETStandard/tests/Opc.Ua.Server.Tests/NodeManager/NodeManagerLifecycleNamespaceRefreshTests.cs
Marc Schier 168b3de353 Replace the RequestManager AsyncLocal guard with an explicit operation context (#4163)
# Description

`RequestManager` tracked whether the calling flow was serving a Client
request in a `private readonly AsyncLocal<bool> m_inServiceDispatch`. It
was set by a `StandardServer.ProcessRequestAsync` override and read by
`NodeManagerLifecycle`, so that a lifecycle operation (`AddAsync` /
`ReloadAsync` / `RemoveAsync`) started from inside a request fails fast
with `InvalidOperationException` instead of draining — and therefore
waiting for — its own request.

Two problems with that mechanism, raised in review of #4093:

1. An `AsyncLocal<T>` written inside an `async` method never flows back
to the caller that awaited it, so every scope had to chain to the
previous value by hand and restore it on dispose. Subtle, and easy to
get wrong when a new entry point is added.
2. Ambient state is invisible at the call site, so the lifetime of the
mark could not be reasoned about from the code that depended on it —
especially once work is handed to a background task.

`RequestManager` also had no business carrying ambient state on behalf
of a different subsystem; its job is tracking requests for the drain.

## What changed

The guard is now an **exact identity lookup** against the request
registry `RequestManager` already maintains, and the operation is
threaded **explicitly**:

* **`RequestManager`** — `internal bool
IsExecutingRequest(IOperationContext?)` performs a reference-identity
lookup against `m_requests` under the existing `m_requestsLock`.
`m_inServiceDispatch`, `EnterServiceDispatchScope()`, the nested
`ServiceDispatchScope` class and the flag-based `IsExecutingRequest`
property are gone.
* **`StandardServer`** — the `ProcessRequestAsync` override is removed;
it existed only to open the dispatch scope.
* **`INodeManagerLifecycle`** — every member (`AddAsync` ×2 factory
kinds, `ReloadAsync` ×2, `RemoveAsync`) now takes the caller's
`IOperationContext?` directly, ahead of `CancellationToken ct =
default`. No overloads and no parallel signature are kept: the interface
is new in 2.0 and outside the 1.5.378 compatibility rule, so call sites
state their context explicitly or pass `null`.
* **`HostedNodeManagerLifecycle`** and
**`RuntimeNodeSetLifecycleExtensions`** thread the argument through;
`AddRuntimeNodeSetAsync` / `ReloadRuntimeNodeSetAsync` remain single
methods.
* **`SystemContextOperationExtensions.GetOperationContext(this
ISystemContext)`** (new, `Opc.Ua.Core`) lets a NodeManager or Method
callback hand over the operation it already received without
downcasting. It switches on `SessionSystemContext` and `SystemContext`,
the two independent roots every context derives from. No new interface
is introduced: casting the context to `IOperationContext` would not do,
because a context implements that by delegating to the operation it was
created for, so the cast yields the context rather than the operation.
* **`NodeManagerLifecycle.EnsureNotRequestCallback`** no longer consults
`ServerState`. The previous `CurrentState == Running` predicate could
let a genuinely re-entrant call slip past while the server was shutting
down; the request registry answers exactly in every state.

A NodeManager or Method callback now reads:

```csharp
await m_lifecycle.ReloadAsync(m_registration, replacement, context.GetOperationContext(), ct);
```

and a control-plane caller (hosted service, DI consumer) passes `null`.

## Behaviour

Unchanged for the case the guard exists to catch: a lifecycle call made
on behalf of an executing request is still rejected up front with the
same `InvalidOperationException` and the same message.

Two deliberate differences:

* **Improvement** — an internal `OperationContext` that was never
enrolled as a Client request is now correctly allowed through. The
boolean flag could not distinguish it from a real request.
* **Accepted trade-off** — a caller inside a request that passes no
operation is no longer detected at entry and falls back to the
pre-existing bounded `RequestManager.RequestDrainTimeout`, which already
covered any request that bypassed the service pipeline. Automatic
detection for an uncooperative caller inherently requires ambient state,
which is exactly what this change removes. Documented in
`docs/NodeManagers.md`.

## Alternatives considered and rejected

| Alternative | Why rejected |
| --- | --- |
| Delete the guard, rely only on `RequestDrainTimeout` | Turns a clear
immediate error into a multi-minute hang. |
| Infer the caller from the `CancellationToken` it passes | Works only
for the exact request token, breaks on linked tokens; ambient magic of a
different kind. |
| In-flight callback counter on `MasterNodeManager` | False positives:
rejects legitimate control-plane calls whenever any request is
concurrently dispatching. |
| Defer the drain to a background task so re-entrancy cannot deadlock |
The drains are woven into transactional commit / rollback and binding
reconciliation; deferring them breaks the transactional guarantees. |
| `Activity.Current` / `ThreadLocal` | The same ambient magic the issue
asks to remove. |

## Testing

* `RequestManagerTests` — the three `EnterServiceDispatchScope` tests
are replaced by coverage for `IsExecutingRequest(context)`: null,
never-registered, only-while-scope-open, visible across an `await` and a
background `Task.Run`, and two concurrently executing requests not
confusing each other.
* `NodeManagerLifecycleTests` — rejection from an executing request for
Add / Reload / Remove without invoking the factory, acceptance for a
context that is not an executing request, acceptance once the request
has completed, and the full callback shape
(`ServerSystemContext.Copy(operation)` → `GetOperationContext()` →
rejected).
* `HostedNodeManagerLifecycleTests` — forwarding tests for the
caller-context argument on all five members.
*
`tests/Opc.Ua.Core.Tests/Stack/State/SystemContextOperationExtensionsTests.cs`
(new) — `GetOperationContext` across both context roots and the
null-argument guard.

## Related Issues

- Fixes #4149

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 14402d11-e540-4faa-a5cd-94881bd7ce85
2026-08-03 20:31:51 +00:00

254 lines
10 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.Globalization;
using System.IO;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using NUnit.Framework;
using Opc.Ua.Client;
using Opc.Ua.Client.ModelChange;
using Opc.Ua.Client.TestFramework;
using Opc.Ua.Server.RuntimeNodeSet;
using Opc.Ua.Server.TestFramework;
using Opc.Ua.Tests;
using Quickstarts.ReferenceServer;
namespace Opc.Ua.Server.Tests.NodeManager
{
/// <summary>
/// Regression coverage for
/// <see href="https://github.com/OPCFoundation/UA-.NETStandard/issues/4100"/>:
/// registering a NodeManager on a running Server appends a namespace
/// uri and bumps <c>UrisVersion</c>. A Client with model change
/// tracking enabled must re-read its namespace table when it observes
/// the resulting model change, otherwise it resolves NodeIds from the
/// new namespace against a stale <c>NamespaceUris</c> table.
/// </summary>
[TestFixture]
[Category("Client")]
[Category("ModelChange")]
[Category("NodeManagerLifecycle")]
[Category("Server")]
[SetCulture("en-us")]
[SetUICulture("en-us")]
[NonParallelizable]
public sealed class NodeManagerLifecycleNamespaceRefreshTests
{
private const string kLiveNamespaceUri =
"urn:opcfoundation.org:Tests:NodeManagerLifecycleNamespaceRefresh";
private const uint kRootNodeId = 8100;
private const string kRootBrowseName = "NamespaceRefreshRoot";
/// <summary>
/// Starts a fresh <see cref="ReferenceServer"/> on opc.tcp and loads a
/// client configuration that shares its PKI root.
/// </summary>
[SetUp]
public async Task SetUpAsync()
{
m_telemetry = NUnitTelemetryContext.Create();
m_pkiRoot = Path.Combine(
TestContext.CurrentContext.WorkDirectory,
nameof(NodeManagerLifecycleNamespaceRefreshTests),
Guid.NewGuid().ToString("N"));
m_fixture = new ServerFixture<ReferenceServer>(t => new ReferenceServer(t))
{
UriScheme = Utils.UriSchemeOpcTcp,
SecurityNone = true,
AutoAccept = true
};
m_server = await m_fixture.StartAsync(m_pkiRoot).ConfigureAwait(false);
m_clientFixture = new ClientFixture(m_telemetry);
await m_clientFixture.LoadClientConfigurationAsync(m_pkiRoot)
.ConfigureAwait(false);
}
/// <summary>
/// Stops the server and cleans up PKI artefacts.
/// </summary>
[TearDown]
public async Task TearDownAsync()
{
m_clientFixture?.Dispose();
m_server?.Dispose();
if (m_fixture is not null)
{
await m_fixture.StopAsync().ConfigureAwait(false);
}
if (!string.IsNullOrEmpty(m_pkiRoot) && Directory.Exists(m_pkiRoot))
{
Directory.Delete(m_pkiRoot, recursive: true);
}
}
/// <summary>
/// A live NodeManager add appends a namespace uri on the Server. A
/// tracking Client must pick the new uri up and be able to resolve a
/// NodeId from it, while a subsequent same-uri reload must leave the
/// Client table unchanged.
/// </summary>
[Test]
[CancelAfter(120_000)]
public async Task LiveNodeManagerAddRefreshesTheClientNamespaceTable(
CancellationToken ct)
{
ManagedSession session = await ConnectTrackingSessionAsync(ct)
.ConfigureAwait(false);
await using (session.ConfigureAwait(false))
{
var refreshed = new TaskCompletionSource<ModelChangedEventArgs>(
TaskCreationOptions.RunContinuationsAsynchronously);
Assert.That(session.ModelChange, Is.Not.Null);
session.ModelChange!.ModelChanged += (_, e) =>
{
if (e.NamespaceTableRefreshed)
{
refreshed.TrySetResult(e);
}
};
int namespaceCountBefore = session.NamespaceUris.Count;
Assert.That(session.NamespaceUris.GetIndex(kLiveNamespaceUri),
Is.EqualTo(-1),
"the live namespace must not be known before the add");
NodeManagerRegistration registration = await m_server
.NodeManagerLifecycle
.AddRuntimeNodeSetAsync(CreateOptions(), null, ct)
.ConfigureAwait(false);
ModelChangedEventArgs observed = await refreshed.Task
.WaitAsync(TimeSpan.FromSeconds(60), ct)
.ConfigureAwait(false);
Assert.That(observed.NamespaceTableRefreshed, Is.True);
Assert.That(session.NamespaceUris.Count,
Is.EqualTo(namespaceCountBefore + 1));
int liveIndex = session.NamespaceUris.GetIndex(kLiveNamespaceUri);
Assert.That(liveIndex, Is.EqualTo(namespaceCountBefore),
"the live namespace uri must be appended to the client table");
// The whole point of the refresh: a NodeId expressed in the
// new namespace must now resolve client side.
var expanded = new ExpandedNodeId(kRootNodeId, 0, kLiveNamespaceUri, 0);
NodeId resolved = ExpandedNodeId.ToNodeId(expanded, session.NamespaceUris);
Assert.That(resolved.IsNull, Is.False);
Assert.That(resolved.NamespaceIndex, Is.EqualTo((ushort)liveIndex));
// A same-uri reload must not grow the table any further.
await m_server.NodeManagerLifecycle
.ReloadRuntimeNodeSetAsync(registration, CreateOptions(), null, ct)
.ConfigureAwait(false);
await Task.Delay(TimeSpan.FromSeconds(2), ct).ConfigureAwait(false);
Assert.That(session.NamespaceUris.Count,
Is.EqualTo(namespaceCountBefore + 1));
await session.CloseAsync(ct).ConfigureAwait(false);
}
}
private async Task<ManagedSession> ConnectTrackingSessionAsync(
CancellationToken ct)
{
var url = new Uri(
Utils.UriSchemeOpcTcp +
"://localhost:" +
m_fixture.Port.ToString(CultureInfo.InvariantCulture));
ConfiguredEndpoint endpoint = await m_clientFixture
.GetEndpointAsync(url, SecurityPolicies.None)
.ConfigureAwait(false);
return await new ManagedSessionBuilder(m_clientFixture.Config, m_telemetry)
.UseEndpoint(endpoint)
.WithSessionName(nameof(NodeManagerLifecycleNamespaceRefreshTests))
.WithSessionTimeout(TimeSpan.FromSeconds(120))
.WithModelChangeTracking()
.ConnectAsync(ct)
.ConfigureAwait(false);
}
/// <summary>
/// Builds the runtime NodeSet options for a model that owns
/// <see cref="kLiveNamespaceUri"/> and contributes a single object
/// organized under the Objects folder.
/// </summary>
private static RuntimeNodeSetOptions CreateOptions()
{
string xml = $"""
<?xml version="1.0" encoding="utf-8"?>
<UANodeSet xmlns="http://opcfoundation.org/UA/2011/03/UANodeSet.xsd">
<NamespaceUris>
<Uri>{kLiveNamespaceUri}</Uri>
</NamespaceUris>
<Models>
<Model ModelUri="{kLiveNamespaceUri}" />
</Models>
<UAObject NodeId="ns=1;i={kRootNodeId}" BrowseName="1:{kRootBrowseName}">
<DisplayName>{kRootBrowseName}</DisplayName>
<References>
<Reference ReferenceType="i=40">i=58</Reference>
<Reference ReferenceType="i=35" IsForward="false">i=85</Reference>
</References>
</UAObject>
</UANodeSet>
""";
return new RuntimeNodeSetOptions
{
Sources =
[
RuntimeNodeSetSource.FromStream(
$"NamespaceRefreshTests-{kLiveNamespaceUri}",
_ => new ValueTask<Stream>(
new MemoryStream(Encoding.UTF8.GetBytes(xml))),
[kLiveNamespaceUri])
]
};
}
private ITelemetryContext m_telemetry;
private string m_pkiRoot;
private ServerFixture<ReferenceServer> m_fixture;
private ClientFixture m_clientFixture;
private ReferenceServer m_server;
}
}