Replace the RequestManager AsyncLocal guard with an explicit operation context

[marcschier]

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:

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 Consider removing AsyncLocal from RequestManager operation context tracking #4149

Checklist

• I have signed the CLA and read the CONTRIBUTING doc.
• I have added tests that prove my fix is effective or that my feature works and increased code coverage.
• I have added all necessary documentation.
• I have verified that my changes do not introduce (new) build or analyzer warnings.
• 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.

[Copilot]

Pull request overview

This PR removes the AsyncLocal-based “in service dispatch” guard from RequestManager and replaces it with an explicit operation-context flow and an identity-based lookup against the existing request registry, so lifecycle operations can deterministically reject re-entrant calls without relying on ambient state.

Changes:

• Replace the ambient AsyncLocal<bool> guard with RequestManager.IsExecutingRequest(IOperationContext?) based on reference identity in the request registry.
• Thread IOperationContext? explicitly through new INodeManagerLifecycle overloads and propagate through hosted lifecycle and runtime NodeSet helpers.
• Introduce IOperationContextProvider + ISystemContext.GetOperationContext() to extract the serving operation from callback contexts, and add/update tests + docs accordingly.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/Opc.Ua.Server.Tests/RequestManagerTests.cs	Updates coverage from EnterServiceDispatchScope to IsExecutingRequest(context) behavior (null/unregistered/active/completed/concurrent).
tests/Opc.Ua.Server.Tests/NodeManager/NodeManagerLifecycleTests.cs	Adds scenarios ensuring lifecycle calls reject when invoked on behalf of an executing request and accept non-request/internal contexts.
tests/Opc.Ua.Server.Tests/Hosting/HostedNodeManagerLifecycleTests.cs	Verifies forwarding of the new caller-context overloads through HostedNodeManagerLifecycle.
tests/Opc.Ua.Core.Tests/Stack/State/SystemContextExtensionsTests.cs	New tests for GetOperationContext() across both SystemContext roots and null-argument behavior.
src/Opc.Ua.Types/State/NodeIdFactorySuppressedContext.cs	Implements IOperationContextProvider to forward the wrapped context’s operation.
src/Opc.Ua.Types/State/ISystemContext.cs	Adds IOperationContextProvider and SystemContextExtensions.GetOperationContext(this ISystemContext).
src/Opc.Ua.Server/Server/StandardServer.cs	Removes the dispatch-scope override and relies on request scope enrollment/identity.
src/Opc.Ua.Server/Server/RequestManager.cs	Removes AsyncLocal + scope type; adds registry identity-based IsExecutingRequest(IOperationContext?).
src/Opc.Ua.Server/RuntimeNodeSet/RuntimeNodeSetLifecycleExtensions.cs	Adds overloads to pass caller operation context through runtime NodeSet lifecycle helpers.
src/Opc.Ua.Server/NodeManager/Lifecycle/NodeManagerLifecycle.cs	Threads caller context through Add/Reload/Remove paths and updates the re-entrancy guard to use the request registry.
src/Opc.Ua.Server/NodeManager/Lifecycle/INodeManagerLifecycle.cs	Adds new overloads that accept IOperationContext? + required CancellationToken.
src/Opc.Ua.Server/Hosting/HostedNodeManagerLifecycle.cs	Implements/forwards the new overloads on the hosted lifecycle wrapper.
src/Opc.Ua.Core/Stack/State/ISessionSystemContext.cs	Makes SessionSystemContext an IOperationContextProvider so callbacks can surface the serving operation.
docs/RuntimeNodeSets.md	Documents the new operation-context overloads and guidance for callback callers.
docs/NodeManagers.md	Updates lifecycle guidance to use explicit operation context instead of ambient detection.