mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-19 07:02:16 +08:00
168b3de353
# 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
254 lines
10 KiB
C#
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;
|
|
}
|
|
}
|