Files
UA-.NETStandard/tests/Opc.Ua.Core.Tests/Stack/State/SystemContextOperationExtensionsTests.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

108 lines
4.2 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 NUnit.Framework;
using Opc.Ua.Tests;
namespace Opc.Ua.Core.Tests.Stack.State
{
/// <summary>
/// Tests for <see cref="SystemContextOperationExtensions"/>, which is how a callback hands
/// the operation it is serving to an API that has to know which operation invoked it.
/// </summary>
[TestFixture]
[Category("SystemContext")]
[SetCulture("en-us")]
[SetUICulture("en-us")]
[Parallelizable]
public class SystemContextOperationExtensionsTests
{
[Test]
public void GetOperationContextThrowsArgumentNullExceptionWhenContextNull()
{
Assert.That(
() => SystemContextOperationExtensions.GetOperationContext(null),
Throws.ArgumentNullException);
}
[Test]
public void GetOperationContextReturnsNullWhenTheSystemContextHasNoOperation()
{
var context = new SystemContext(NUnitTelemetryContext.Create());
Assert.That(context.GetOperationContext(), Is.Null);
}
[Test]
public void GetOperationContextReturnsNullWhenTheSessionSystemContextHasNoOperation()
{
var context = new SessionSystemContext(NUnitTelemetryContext.Create());
Assert.That(context.GetOperationContext(), Is.Null);
}
[Test]
public void GetOperationContextReturnsTheOperationASystemContextWasCopiedFor()
{
ITelemetryContext telemetry = NUnitTelemetryContext.Create();
var operation = new SystemContext(telemetry);
var context = new SystemContext(telemetry);
ISystemContext copy = context.Copy(operation);
Assert.That(copy.GetOperationContext(), Is.SameAs(operation));
}
[Test]
public void GetOperationContextReturnsTheOperationASessionSystemContextWasCopiedFor()
{
// ServerSystemContext derives from SessionSystemContext, so this is the shape a
// NodeManager or Method callback actually receives.
ITelemetryContext telemetry = NUnitTelemetryContext.Create();
var operation = new SystemContext(telemetry);
var context = new SessionSystemContext(telemetry);
ISystemContext copy = context.Copy(operation);
Assert.That(copy.GetOperationContext(), Is.SameAs(operation));
}
[Test]
public void GetOperationContextReturnsTheOperationASessionSystemContextWasBuiltWith()
{
ITelemetryContext telemetry = NUnitTelemetryContext.Create();
var operation = new SystemContext(telemetry);
var context = new SessionSystemContext(operation, telemetry);
Assert.That(context.GetOperationContext(), Is.SameAs(operation));
}
}
}