mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-28 22:55:49 +08:00
ac29b51048614101da9c1dcb5cc55991c43006fb
2779
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
ac29b51048 |
Remove experimental markers from type interfaces (#4785)
# Description Remove `UA_NETStandard_1` from `IType`, `IBuiltInType`, `IEnumeratedType`, and `IEncodeableType`. Add a regression test to guard their public metadata. - **Follow-up candidates:** `Variant.FromEnumeration`, `AmbientMessageContext`, `Utils.ParseExtension`/`UpdateExtension`, and `LoggerUtils` retain `UA_NETStandard_1` and are not changed here. - **Retained:** `OPCUA_EXPERIMENTAL_KC_BRIDGE` for the intentionally experimental KeyCredential bridge. ## Related Issues ## Checklist - [ ] 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. - [ ] I have added all necessary documentation. - [x] 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. <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes #4784 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
Cristi PogaceanandCopilot
|
af601ebca8 |
Complete model-change reporting for committed node deletions (#4778)
# Description Finalize deletion model-change reporting from observed address-space outcomes, including when later cleanup fails or the caller cancels the request. A shared internal snapshot captures registered subtree nodes, pre-removal NodeVersion eligibility, and relevant reference endpoints. Completion reports only removed nodes and references actually removed from surviving local nodes, then rethrows the original deletion failure. If reporting also fails, both errors remain visible. This preserves notifications for versioned descendants below unversioned roots and applies strict deletion eligibility to the service-facing overload. It avoids blindly reporting the requested subtree from a finally block. For endpoints owned by another node manager in the same server, reporting requires target-reference cleanup to be both requested and successful because the remaining foreign references cannot be inspected locally. A successful service deletion with `DeleteTargetReferences = false` does not imply foreign reference removal. Reporting covers both directions of hierarchical and non-hierarchical references; absolute targets are excluded, matching the cleanup path. Master cleanup now propagates a bad per-reference `DeleteReferenceAsync` result as a `ServiceResultException`. A normal return with a bad status is therefore not mistaken for successful foreign cleanup. Committed local changes are still reported before the cleanup failure is rethrown; unconfirmed foreign reference removals are not reported. The branch includes the merge of current master after #4776 and #4777 merged. This PR preserves their per-node monitored-item restoration, independent cleanup budgets, and NodeVersion eligibility behavior. It does not introduce further monitored-item completion changes; the remaining diff is outcome-based model-change completion, its regression tests, and documentation. It contains no release-version changes. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4769 - Related: #4768, #4770 (addressed by merged PRs #4776 and #4777). - Design and failure reproduction are recorded in #4769. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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. ## Validation ### Current head: `524e46fbe` The server test project and its dependencies built without build/analyzer warnings or errors. The following command covers the deletion/model-change regressions, the complete monitored-item lifecycle fixture, and the master node-management dispatcher fixture: ```powershell & 'C:\Program Files\dotnet\dotnet.exe' exec 'C:\Program Files\dotnet\sdk\10.0.402\dotnet.dll' test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-restore -f net48 -p:CustomTestTarget=net48 -p:NuGetAudit=false -m:2 --filter "FullyQualifiedName~AsyncCustomNodeManagerTests&Name~Delet|FullyQualifiedName~AsyncCustomNodeManagerNodeManagementTests&Name~DeleteNode|FullyQualifiedName~MonitoredItemManagerLifecycleTests|FullyQualifiedName~MasterNodeManagerNodeManagementTests" --logger "trx;LogFileName=foreign-cleanup-status-net48.trx" --results-directory 'C:\Users\crpogace\.copilot\session-state\f4f431fc-413e-40d4-88a7-83f6ade0c9a7\files\foreign-cleanup-status-results' ``` Result: **296 passed, 0 failed**; 25 fixture combinations are inapplicable to the legacy adapter and were not executed. Regression coverage includes late cleanup failure/cancellation, pre-mutation cancellation, partial subtree removal, service eligibility, versioned descendants, preservation of simultaneous deletion/reporting failures, foreign-reference direction/type combinations, requested versus omitted cleanup, failed cleanup, and absolute-target exclusion. New regressions exercise the real master cleanup dispatcher with `Good`, `BadNotSupported`, and `BadUnexpectedError` results across async internal/service deletion and legacy runtime deletion. Before the fix, 10 bad-status cases failed; the 5 successful-cleanup controls passed. **Current-head net10.0 tests were not executed locally:** the machine now has SDK 10.0.402 rather than the pinned 10.0.401. Restore requires `Microsoft.NET.ILLink.Tasks` 10.0.13, which is absent from the local cache, and the NuGet endpoint fails its TLS handshake. This is an environment/restore blocker, not a test failure. The explicit SDK invocation above does not change the repository SDK pin. `NuGetAudit=false` was invocation-only because the audit endpoint was unavailable; repository audit settings are unchanged. The earlier pre-merge revision `0c53e143b` passed 96 focused deletion tests on each of net10.0 and net48, with 22 inapplicable fixture combinations per framework. Those historical results do not establish net10.0 validation of the current merged head. Full net48/net10.0 solution validation has not been completed locally. The ordinary .NET 10 VSTest invocation cannot run the unchanged AOT projects' Microsoft.Testing.Platform suites; this is a mixed-runner invocation limitation, not evidence of an AOT compilation defect. Their supported validation path is native publish-and-run, as used in CI. A separate local Historian native AOT run passed 2/2 tests earlier in this work, but does not establish complete AOT or current-head validation. CI was still running when this description was updated; the full-suite and CI checklist items remain unchecked. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
romanettandClaude Opus 5.5
|
5b03e9afa4 |
Channel send path: seal responses in the writer, lock-free gate, fewer locks and allocations per request (#4782)
Carved out of #4771 (roadmap item A1 of the o6 benchmark gap analysis): everything except running requests to completion on the receive loop. #4771 keeps run to completion, stacked on this PR, for a later release. Requests still go through the fair request queue and its workers, as on master. What changes is the channel around them: - the response is encoded without the channel gate; - the channel's single writer numbers, secures and sends; - requests take fewer locks and allocations on the channel. No wire, configuration or public API change. ## Results Paired A/B against master `6cfb8cf19` (with #4767). The builds alternate per round on the same machine, and the table shows medians of 5 rounds with the throughput range of the rounds in brackets. The machine was shared with other work, so the CPU and throughput ranges are wide; allocated bytes per request are deterministic. - **o6read** uses the native open62541 client workers of the o6-automation benchmark, in the o6 2.0.1 configuration: `AsyncCustomNodeManager`, Server GC, queue = clients × outstanding + 3. - **"4 cores"** pins the server to 4 cores. - **The other rows** use the harness's .NET client: 20 sessions unless noted, workstation GC, ReferenceServer. - **Contention** counts runtime lock-contention events in the measured window. | Scenario | req/s | Server CPU µs/req | Alloc B/req | Contention | |---|---|---|---|---| | o6read 10×100, 24 cores | 254,600 → 264,700 (+4 %) [228–292k → 255–270k] | 67.4 → 69.5 | 5,874 → 4,709 (**−20 %**) | 6,766 → 1,811 | | o6read 10×1000, 24 cores | 248,900 → 241,300 (−3 %) [231–258k → 233–260k] | 69.0 → 71.3 | 5,866 → 4,718 (**−20 %**) | 6,080 → 1,456 | | o6read 10×100 encrypted, 24 cores | 203,900 → 232,000 (**+14 %**) [111–226k → 204–264k] | 71.6 → 70.2 | 5,985 → 4,752 (**−21 %**) | 5,166 → 1,288 | | o6read 10×100 `--o6-async 0` (`CustomNodeManager2`), 24 cores | 235,600 → 243,900 (+4 %) | 40.1 → 38.8 | 6,702 → 5,886 (−12 %) | 148k → 124k | | o6read 1 client × 1000 (one channel) | 64,300 → 92,600 (**+44 %**) [52–69k → 76–100k] | 79.8 → 74.5 (−7 %) | 6,003 → 5,063 (−16 %) | 224 → 128 | | o6read 10×100, 4 cores | 156,600 → 140,400 (−10 %, noise: rounds 139–192k → 121–184k) | 25.0 → 28.3 | 5,435 → 4,688 (−14 %) | 97 → 184 | | o6read 10×1000, 4 cores | 107,100 → 125,200 (**+17 %**) [103–116k → 120–136k] | 36.5 → 31.7 (**−13 %**) | 9,966 → 4,767 (**−52 %**) | 312 → 160 | | o6read 10×100 encrypted, 4 cores | 103,300 → 126,300 (**+22 %**) [102–105k → 115–143k] | 37.9 → 31.3 (**−17 %**) | 5,498 → 4,736 (−14 %) | 46 → 29 | | o6read 10×100 `--o6-async 0`, 4 cores | 186,600 → 187,700 (+1 %) | 21.1 → 21.0 | 6,587 → 5,915 (−10 %) | 17.7k → 4.2k | | read --nodes 1, 20 sessions | 208,300 → 205,900 (−1 %) | 49.1 → 48.3 | 4,905 → 4,621 (−6 %) | p50/p99 0.072/1.18 → 0.072/1.26 ms | | read --nodes 1, 200 sessions | 216,400 → 208,600 (−4 %) | 42.0 → 43.2 | 4,790 → 4,518 (−6 %) | p50/p99 0.606/3.56 → 0.615/4.05 ms | | read --nodes 100 | 31,950 → 32,470 (+2 %) | 278 → 253 (−9 %) | 73,702 → 73,436 | p50/p99 0.290/2.51 → 0.283/2.48 ms | | browse | 38,380 → 40,460 (+5 %) | 162 → 161 | 39,305 → 38,981 | p50 0.349 → 0.330 ms | | sub (5 × 100 items, 100 ms) | 88,190 → 89,010 notif/s | 11.1 → 10.7 | 159 → 153 | p50/p99 20.7/64.3 → 20.8/66.0 ms | **Where it helps:** - Allocation per request falls in every scenario, by 20 % for o6read. - Lock contention falls by 70–80 % for o6read. - Throughput rises where the channel gate serialised the work: - one deep pipeline on one channel: +44 %; - encrypted: +14 % on 24 cores, +22 % on 4 cores; - 1000 in flight on 4 cores: +17 %. **Where it does not:** CPU per request at 24 cores is unchanged. The requests still go through the fair queue's workers, and the hand-off to a worker and back is what #4771 removes. The 4-core 10×100 row spread as much within each side as between the sides. ## Changes ### Sending - **Seal in the writer.** The producer encodes the response without the channel gate and queues it unsealed. The channel's single writer takes the queued responses in order under a per-channel send-order lock: it assigns sequence numbers, selects the current token, signs and encrypts, and sends the batch. Messages that number themselves (OpenSecureChannel, asymmetric faults, the client's requests) hold the same lock until they are queued. Token changes that can dispose a token take it as well. - **Reconnect.** The old transport is detached before the new token is activated. Unsealed responses still queued for it are then dropped without taking sequence numbers. The new transport is attached and the OpenSecureChannel response numbered under one hold of the send-order lock, so the response is the first message on the new transport. The channel opens again only after that. - **Failure past the sequence numbers.** A message whose security fails after its chunks took sequence numbers stops all numbering on that transport. The writer drops the messages producers numbered for it after the gap, and reports a transport error once it has left the send-order lock. That faults the channel (server) or reconnects (client). - **Numbered writes without the lock.** The writer takes writes that their producers numbered without the send-order lock, because they are queued in number order. A client channel queues only such writes, so its writer no longer waits for the lock that its request threads take. - **Writer on the pool.** The writer always starts on the thread pool. Running it on the producer's thread sent batches of one message. - **Buffers.** The encoded message is a struct in the write-queue entry. A small single-chunk message moves to a right-sized buffer only when writes are already queued, so deep pipelines no longer exhaust the bounded buffer pool (−52 % allocation at 10×1000 on 4 cores). ### Locks and allocations per request - **`ChannelGate`** takes the gate with one compare-and-exchange when it is free, and queues waiters under a lock only when it is not. `SemaphoreSlim` took its monitor on every wait and release, which the receive loop does for every message. The contract is unchanged: not re-entrant, FIFO handoff, cancellable waits, synchronous completion when free. The transport's send lock is a `ChannelGate` too. - **Namespace and server tables.** Every encoder and decoder reads `ServiceMessageContext.NamespaceUris`, `ServerUris` and `MappingVersion`. They are now read lock-free from one immutable snapshot that setters swap atomically. - **Allocation-free tracking.** Tracking a pending request (`PendingRequestScope`) and handing a decoded request out of the gate allocate nothing. - **Single-chunk decode.** A request in one chunk is decoded from that chunk's buffer. - **Fair-queue entry.** An entry coordinates its cancellation registration with an interlocked state (`RegistrationState`) instead of a lock object. ### Client fix `Session.CloseAsync` disposed the session synchronously, and `Dispose` waits for the async dispose of the session and its subscription engine. Closing 1000 sessions at once blocked one pool thread per session while that dispose needed pool threads, and the close took minutes in `ServerManySessionsLoadTestAsync(1000)`. `CloseAsync` now awaits `DisposeAsync`, and dispose awaits an engine that implements `IAsyncDisposable`, as `DefaultSubscriptionEngine` now does. ## Behaviour notes - **Wire order and tokens.** Sequence numbers stay strictly increasing in wire order, and the chunks of one message stay contiguous. Token renewal applies at a message boundary. After a reconnect, the OpenSecureChannel response is the first message on the new transport. - **Back-pressure** (#4707) is unchanged. The receive loop stops reading above 100 queued writes. - **Client.** `Session.CloseAsync` no longer blocks a thread on the dispose. ## Not in this PR The striped server-stop tokens of #4771 are dropped. Since #4767 a server request no longer registers with the stop token, because the deadline monitor polls it. ## Testing - `UaSCBinaryChannelSendOrderTests` covers: - wire order and random interleaving; - token renewal (decrypt and verify) for five policies; - buffers returned once; - a seal failure after numbering stops the channel, and a message numbered after the gap is not sent; - the replaced transport; - back-pressure; - a response that cannot be encoded is answered with a fault. - `TcpReconnectKeyAgreementTests.AResponseQueuedForTheDroppedTransportIsNotSecuredWithTheNewTokenAsync` (new) holds the old transport's sends during a reconnect. It fails on the previous order. The seal-failure test fails without the gap check. - Also run: `ChannelGateTests`, `TcpByteTransportTests`, `ServiceMessageContextMappingTests`. - Local, net10.0: - Opc.Ua.Core.Tests (Stack.Server, Transport, Tcp, ChannelGate): 2739 passed, 0 failed; - Opc.Ua.Configuration.Tests: 241 passed, 0 failed. net48 builds. `dotnet format whitespace --verify-no-changes` passes on the changed files. - CI: the `CI` workflow on this head is running; the commits were tested together with run to completion in #4771 (its last full CI run passed). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
2f72239a72 |
Quickstarts alarms: report one event per Enable instead of two (#4780)
Every Enable call on a reference-server alarm produced **two** condition
events (different EventIds, same Time); Disable produced one.
## Cause
`ConditionTypeHolder.OnEnableDisableAlarm` is registered as the
condition's `OnEnableDisable` hook, which
`ConditionState.ProcessBeforeEnableDisable` calls *before*
`OnEnableCalled`/`OnDisableCalled` change the `EnabledState`. The hook
switched the state (`SetEnableState`) and called `ReportEvent()` itself;
the core then ran `UpdateStateAfterEnable` and `ReportStateChange` for
the same change, because every concrete holder sets
`AutoReportStateChanges` (`AcknowledgeableConditionTypeHolder`). On
Disable the hook's `ReportEvent` is skipped (the condition is already
disabled), so only Enable doubled.
Part 9 §5.5.2: a change of `EnabledState` causes a ConditionType Event
Notification — one per change. The duplicate also made the CTT's *A and
C Enable Test_002* flaky: it took the second notification as the answer
to its next call (worked around in the CTT scripts).
## Fix
The hook keeps setting the `Message` ("Enabling" / "Disabling alarm …",
carried by the core's event) and leaves the state change and its event
to the core. For a condition without `AutoReportStateChanges` it still
changes the state and reports, as before.
## Tests
`AlarmsAndConditionsEnableTests.DisableEnableViaTypeAndInstanceMethodNoEventsDuringDisabledAsync`
now asserts exactly one condition event in the Enable call window —
fails on master (`Expected: 1 But was: 2`), passes with the fix. All A&C
history tests (152, 3 runs) and client alarm tests (55) pass.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
romanettandClaude Opus 5.5
|
d90ff9fd8f |
Server: release the HTTPS listener's pinned TLS certificate when it fails to open (#4775)
## Problem A full local `Opc.Ua.Server.Tests` run (net10.0) passed all 8606 tests, but the assembly-level certificate leak check failed the teardown: ``` Certificate leak detected: 1 instance(s) created but not disposed UNREACHABLE UNDISPOSED CERTIFICATES: at Certificate.AddRef() at HttpsTransportListener.PrepareTlsCertificate() at HttpsTransportListener.StartAsync / OpenAsync at ServerBase.CreateServiceHostEndpointAsync ... at ServerStartupTests.StartServerAsync(String uriScheme) (https case) ``` It did not reproduce when `ServerStartupTests` ran on its own. ## Root cause `HttpsTransportListener.StartAsync` pins the TLS certificate (`PrepareTlsCertificate` takes an `AddRef` handle) **before** Kestrel binds the port. If the port is already taken, Kestrel's `IOException` escapes `OpenAsync`. `ServerBase.CreateServiceHostEndpointAsync` logs and rethrows it **without disposing the listener**. The listener is not in `TransportListeners` yet, so nothing ever releases it. `ServerFixture` then retries on another port and the test passes, but the pinned certificate is lost. Ports collide when parallel suites race for the port `GetNextFreeIPPort` hands out, which is why the leak only showed up under load. ## Fix - **`ServerBase.CreateServiceHostEndpointAsync`** disposes a listener that fails to open. Only listeners in `TransportListeners` are disposed when the server stops, so a failed one has no other owner. The `listener` parameter doc now states that the server takes ownership. - **`HttpsTransportListener.StartAsync`** stops the listener again when the start fails. That releases the pinned certificate and any host it built, so any caller that drops a listener after a failed open leaks nothing. If the cleanup also fails, both errors are reported in an `AggregateException`, the same pattern as `ServerBase.CompleteServerStartAsync`. Either change alone fixes the reported leak. Both are kept: the first fixes the ownership gap for every transport, the second makes the HTTPS listener clean up after itself whoever the caller is. ## Tests - `ServerStartupTests.StartServerOnTakenPortRetriesOnAnotherPortAsync(https | opc.https)`: holds the port with a dual-mode `TcpListener`, so the first start fails and the fixture retries. **Before the fix**, the assembly leak check reported 2 leaked handles (one per scheme) with the exact creation stack above. **After the fix**, it is clean. - opc.tcp is not covered: on Windows the UA TCP listener binds even while another socket holds the wildcard port, so its start does not fail. - `ServerStartupTests.CreateServiceHostEndpointDisposesListenerThatFailedToOpenAsync`: mock listener whose `OpenAsync` throws; checks that `DisposeAsync` is called once and the original exception is rethrown. - `HttpsTransportListenerTests.OpenOnTakenPortReleasesPinnedTlsCertificateAsync`: real listener and certificate on a taken port; checks that the pinned certificate fields are cleared after the failed open. Each unit test fails with only its own fix reverted. With either fix alone, the end-to-end test leaves the leak check clean. ## Local verification (net10.0) - `Opc.Ua.Server.Tests` `ServerStartupTests`: 8/8 passed, leak check clean. - `Opc.Ua.Core.Tests` `HttpsTransportListenerTests`, `WssAdmissionTests`, `SharedKestrelHost*`: 74/74 passed. This includes the "partially opened listener" tests, which rely on a failed `OpenAsync`. - The full `Opc.Ua.Server.Tests` run was stopped partway through, with no failures up to that point. net48 was not built locally; CI covers the full suites and the other target frameworks. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
b36cacbeac |
ManagedSession: skip redundant outer reconnect after channel-manager recovery (fixes BoundClientCannotRetryAcrossFaultedManagedEntryReplacementAsync flake) (#4781)
## Problem `SessionBindingLiveTests.BoundClientCannotRetryAcrossFaultedManagedEntryReplacementAsync` (added in #4752) failed once on CI (`test-windows-net10.0 (2/15)` of #4775, run 38508438526): ``` Assert.That(lease.State, Is.EqualTo(ChannelState.Ready)) Expected: Ready But was: TransportReconnecting ``` ## Root cause Only the `ManagedSession` fixture variant (`[TestFixture(true)]`) fails; the plain-`Session` run in the same job passed. From the failing run's log: - 28.476: the budgeted reconnect is exhausted and the channel goes `Faulted`. `ManagedSession.OnManagedChannelStateChanged` calls `TriggerReconnect`, so the outer `ConnectionStateMachine` takes over by design: `Reconnect attempt 0, delay 1025 ms`. - 28.976 – 29.502: the test's manual `ReconnectAsync` swaps in a fresh entry and runs its reconnect cycle, which reactivates the session. `Reconnect completed ... Outcome=success` is logged and the state is `Ready`. - 29.502: the outer backoff expires (`ManagedSession: Reconnecting.`). `HandleReconnectAsync` runs a full transport reconnect on the channel that was just recovered, and the state goes `Ready -> TransportReconnecting` just before the test reads `lease.State`. `ClientChannelManager.ReconnectAsync` behaves correctly: it returns only after its own swap and cycle have completed. The defect is the redundant outer reconnect. It disconnects a freshly reactivated channel and costs an extra transport connection (which could also break the test's "no retry" connection-count assertion). ## Fix **`ManagedSession`** - Count channel-driven session reactivations: `Ready` transitions out of `TransportConnectedSessionReactivating` on this session's lease. A participant that failed fatally is removed from the entry before `Ready`, so it is never counted. - When a channel fault (`Faulted`/`Closed`) or a keep-alive failure requests the outer reconnect, record the current count as the trigger mark. - `HandleReconnectAsync` (under the recovery writer lock) skips the reconnect and reports success if a reactivation happened after the mark, the channel is still `Ready` and no channel recovery is in progress. It logs `ManagedSession: Reconnect skipped; the channel manager already reactivated the session.` (`ManagedSession + 30`). - Explicit `ManagedSession.ReconnectAsync` requests are counted separately (requested vs. served by a completed reconnect that started after them). While one is outstanding, the outer reconnect is never skipped, whatever automatic triggers happen meanwhile. A trigger the state machine rejects clears the mark, and overwriting the mark with a later trigger only makes skipping less likely. **Tests** - New `ManagedSessionSkipsOuterReconnectAfterChannelManagerReactivationAsync` (managed variant only). A gated `IReconnectPolicy` holds the outer reconnect in its backoff while the test faults the lease, recovers it through the manager and takes a connection snapshot; it then releases the gate, waits for `Connected`, and asserts no extra transport connection and a working session. No wall-clock dependency. Without the skip it fails with one extra connection. - New `ExplicitManagedReconnectRunsAfterChannelManagerReactivationAsync`: an explicit `ReconnectAsync` joins the gated outer reconnect, a later fault re-triggers it, and the manager recovers the channel. The explicit reconnect must still open a new connection. It failed (`Expected: greater than 9, But was: 9`) before explicit requests were tracked separately. - `BoundClientCannotRetryAcrossFaultedManagedEntryReplacementAsync`: in the managed variant, wait for the outer recovery to start before the manual reconnect, and await `StateMachine.WaitForConnectedAsync` before asserting `Ready` and taking the connection-count snapshot. - `WithSessionAsync` takes an optional `IReconnectPolicy`; new helpers `FaultLeaseAsync` and `GatedReconnectPolicy`. ## Verification - Injected `await Task.Delay(200)` after the reconnect cycle in `ClientChannelManager.ReconnectLeaseAsync` (not committed). The old test then fails every time with the exact CI assertion, and the updated test passes. - `CustomTestTarget=net10.0 dotnet test -c Release -f net10.0 --filter FullyQualifiedName~SessionBindingLiveTests`: 5 sequential runs, 94/94 passed each time. These runs had the test change only; the `ManagedSession` skip was not yet in place. - With the skip: the new test and the original test pass; with the skip disabled, the new test fails as described above. - `--filter "FullyQualifiedName~ManagedSession|FullyQualifiedName~SessionBindingLive|FullyQualifiedName~Reconnect"` on `Opc.Ua.Client.Tests` (net10.0, Release) with the final code: 514 passed, 2 skipped (the managed-only tests in the plain-`Session` fixture). Full `Opc.Ua.Client.Tests` / `Opc.Ua.Sessions.Tests` runs and the net48 build are left to CI. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Cristi PogaceanandCopilot
|
27c17fb410 |
Preserve NodeVersion eligibility when deleting runtime nodes (#4777)
# Description Preserve a runtime node's NodeVersion eligibility before removing it from the address space. Previously, deletion removed the local node before model-change filtering, which then treated it as an unresolved external node and emitted a spurious NodeDeleted entry. This fixes unversioned runtime/session-diagnostics deletion in both AsyncCustomNodeManager and the legacy CustomNodeManager. Strict mode suppresses unversioned deletion entries; versioned nodes and the explicit compatibility mode keep their events. The regression matrix exercises all four eligibility combinations across both async configurations and the legacy adapter. The documentation now cites OPC UA Part 3 section 9.32.2, which requires NodeVersion/model-change correlation. This PR is intentionally limited to the original eligibility bug. Per-node monitoring completion and outcome-based model-change completion are separate changes tracked in #4768 and #4769, including broader subtree/service-deletion handling. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4770 - Related: #4768, #4769 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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. ## Validation The server test project and its dependencies built successfully on net10.0 and net48 with no new build/analyzer warnings. Focused reruns: ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net10.0 -p:CustomTestTarget=net10.0 --filter "Name~DeleteNodePreservesModelChangeEligibilityAsync" ``` Result: **12 passed, 0 failed**. ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net48 -p:CustomTestTarget=net48 --filter "Name~DeleteNodePreservesModelChangeEligibilityAsync" ``` Result: **12 passed, 0 failed**. Full-solution .NET 10 validation was attempted, but the unchanged AOT test projects reject the VSTest target with Microsoft.Testing.Platform 2.4.1 under SDK 10.0.401. The run was stopped after this blocker was confirmed. It is not reported as a passing suite. Exact isolated reproduction of the blocker: ```powershell dotnet test tests\Opc.Ua.Aot.Tests\Opc.Ua.Aot.Tests.csproj --no-build --no-restore -p:CustomTestTarget=net10.0 -p:NuGetAudit=false ``` Result: **failed** with "Testing with VSTest target is no longer supported by Microsoft.Testing.Platform on .NET 10 SDK and later." The full net48 solution was not run. Restore/build commands used `NuGetAudit=false` because NuGet's audit endpoint was unavailable; repository audit settings are unchanged. Published as a draft by request until the full validation/CI gates are satisfied. No release-version files are included; this branch starts from master. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Cristi PogaceanandCopilot
|
f1e51c19f1 |
Restore monitoring for survivors of partial node deletion (#4776)
# Description Complete failed recursive deletion according to each monitored node's actual state, rather than inferring the entire subtree's outcome from the root. Monitored items for removed nodes remain detached and source-deleted. Items for surviving descendants are restored with their existing identity and ownership. Restoration attempts continue after individual failures, and compensation failures are surfaced together with the original deletion failure. The change covers internal and service-facing async deletion and the synchronous compatibility path. Regression tests monitor both the removed root and surviving child across both monitored-item implementations. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4768 - Related: #4769, #4770 - Design and failure reproduction are recorded in #4768; this draft awaits review/sign-off. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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. ## Validation The server test project and its dependencies built successfully on net10.0 and net48 with no new build/analyzer warnings. The entire monitored-item lifecycle fixture, including six new partial-subtree cases and four new compensation-failure cases, passed: ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~MonitoredItemManagerLifecycleTests" ``` Result: **63 passed, 0 failed**. ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net48 -p:CustomTestTarget=net48 --filter "FullyQualifiedName~MonitoredItemManagerLifecycleTests" ``` Result: **63 passed, 0 failed**. The full-solution .NET 10 VSTest invocation is blocked by the unchanged AOT test projects' Microsoft.Testing.Platform configuration. Full net48/net10 solution validation and CI remain outstanding. The blocker was reproduced in the original-fix worktree with: ```powershell dotnet test tests\Opc.Ua.Aot.Tests\Opc.Ua.Aot.Tests.csproj --no-build --no-restore -p:CustomTestTarget=net10.0 -p:NuGetAudit=false ``` Result: **failed** because Microsoft.Testing.Platform 2.4.1 rejects VSTest under SDK 10.0.401. Restore/build commands used `NuGetAudit=false` because the audit endpoint was unavailable; repository audit settings are unchanged. This is an independent master-based draft. It does not include the separate NodeVersion eligibility fix or the model-change completion redesign. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Cristi PogaceanandCopilot
|
89c1275d9d |
Fix managed browse closed-channel and cancellation handling (#4774)
# Description Fix managed browse failure reporting when the session's channel has closed or the caller has already cancelled the operation. - Use the existing checked `TransportChannel` accessor in `ClientBase.MessageContext`, so a closed channel reports `ServiceResultException(BadSecureChannelClosed)` instead of `NullReferenceException` during `Browser` construction. - Check cancellation at entry to `SessionClientExtensions.ManagedBrowseAsync` and the array-based `Browser.BrowseAsync`, before session access or request submission. - Document failure semantics and add unit plus real client/server regression tests for closed channels, pre-cancelled empty/non-empty inputs, Browse/BrowseNext request failures and cancellation, and mixed per-node results. - Preserve existing propagation of request exceptions and the per-node result/error contract. No dependency, analyzer configuration, or public API signature changes. The real-session before-fix run reproduced three failures (closed channel, and pre-cancelled closed channel with empty/non-empty input). All corresponding regressions pass after the fix. ## Related Issues Fix the closed-channel and pre-cancelled managed-browse behavior described in the linked bug report. _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4773 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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. ## Validation Windows, .NET SDK 10.0.401. **134 targeted test cases passed**, including existing browser and continuation-point regressions: | Suite | net10.0 | net48 | | --- | ---: | ---: | | BrowserTests + BrowserAuditRegressionTests | 27 passed | 27 passed | | ClientBaseTests | 18 passed | 18 passed | | ContinuationPointInBatchTest managed-browse cases | 22 passed | 22 passed | | Total | **67 passed** | **67 passed** | Tests were added and prove the fix, but coverage percentages were not collected; the combined tests/coverage checklist item is therefore left unchecked. CLA status has not been independently verified. ### Exact test commands and results Commands ran from the repository root. Test output was redirected to local logs. ```powershell # PASS: 27 tests. dotnet test Tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj --no-restore -f net10.0 -p:CustomTestTarget=net10.0 -p:NuGetAudit=false '-p:WarningsNotAsErrors=CA2000%3BCA2007' --filter "FullyQualifiedName~Opc.Ua.Client.Tests.BrowserTests|FullyQualifiedName~BrowserAuditRegressionTests" --logger "console;verbosity=normal" # PASS: 18 tests. dotnet test Tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj --no-restore -f net10.0 -p:CustomTestTarget=net10.0 -p:NuGetAudit=false '-p:WarningsNotAsErrors=CA2000%3BCA2007' --filter "FullyQualifiedName~ClientBaseTests" --logger "console;verbosity=normal" # PASS: 22 tests, using the freshly rebuilt net10.0 test binaries. dotnet test Tests\Opc.Ua.Sessions.Tests\Opc.Ua.Sessions.Tests.csproj --no-build --no-restore -f net10.0 --filter "FullyQualifiedName~ContinuationPointInBatchTest&Name~ManagedBrowse" --logger "console;verbosity=normal" # PASS: 27 tests. dotnet test Tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj -f net48 -p:CustomTestTarget=net48 -p:NuGetAudit=false '-p:WarningsNotAsErrors=CA2000%3BCA2007' --filter "FullyQualifiedName~Opc.Ua.Client.Tests.BrowserTests|FullyQualifiedName~BrowserAuditRegressionTests" --logger "console;verbosity=normal" # PASS: 18 tests. dotnet test Tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -f net48 -p:CustomTestTarget=net48 -p:NuGetAudit=false '-p:WarningsNotAsErrors=CA2000%3BCA2007' --filter "FullyQualifiedName~ClientBaseTests" --logger "console;verbosity=normal" # PASS: 22 tests. dotnet test Tests\Opc.Ua.Sessions.Tests\Opc.Ua.Sessions.Tests.csproj -f net48 -p:CustomTestTarget=net48 -p:NuGetAudit=false '-p:WarningsNotAsErrors=CA2000%3BCA2007' --filter "FullyQualifiedName~ContinuationPointInBatchTest&Name~ManagedBrowse" --logger "console;verbosity=normal" ``` ### Production build and patch checks ```powershell # PASS: production Client/Core dependency build, 0 warnings and 0 errors. # Normal warnings-as-errors policy; no analyzer downgrades. dotnet build src\Opc.Ua.Client\Opc.Ua.Client.csproj --no-restore -f net48 -p:CustomTestTarget=net48 -p:NuGetAudit=false --verbosity minimal # PASS: no whitespace errors. git diff --check ``` All six changed files retain CRLF line endings. No build/analyzer diagnostics were reported in the changed files. ### Validation limitations - Initial unmodified-policy test builds failed on unavailable NuGet audit data (`NU1900`) and unrelated existing `CA2000`/`CA2007` diagnostics in test code. - The test commands use the documented offline `NuGetAudit=false` option and command-only analyzer downgrades. TODO: remove those command-only downgrades when the existing certificate/transport test diagnostics are fixed. Repository build/analyzer settings are unchanged. - An initial full `ContinuationPointInBatchTest` fixture invocation rebuilt the net10.0 binaries but was stopped during expensive existing node-cache traversals. It was replaced by the passing managed-browse subset above. - The complete `UA.slnx` suites were **not run**. Full-solution validation, coverage measurement, and CI/CodeQL results remain outstanding. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
romanettandClaude Opus 5.5
|
6cfb8cf196 |
Server: remove per-request context copies and lock hot spots from the request path (−16 % allocation per Read) (#4767)
## Description Five small changes to the per-request server path, found in the CPU and allocation profiles of the o6 read benchmark (`AsyncCustomNodeManager`, Server GC, 10 clients × 100 Reads in flight) on master |
||
|
Marc Schier
|
be50f02700 |
Add additional companion spec MCP tool profiles (#4731)
# Description Expose existing stack APIs through **337 additional domain MCP tools**: 114 industrial-companion tools, 35 WoT/xRegistry/document tools, and 188 higher-level stack-workflow tools. The fully enabled catalogue contains **492 tools**; profile selection is not an execution grant. The packages now follow the reviewed structure: **`Opc.Ua.Mcp.Workflows`** contains roles/users, files, alarms and history; **`Opc.Ua.Mcp.WotCon`** contains management/documents and independently registered live WoT execution. GDS and schema tools remain separate packages. The executable composes **18 libraries**. | Workflow | Added domain tools | Profiles | | --- | ---: | --- | | GDS directory, certificates, reference-only credentials/tokens and Part 21 onboarding | 46 | `gds`, `certificates`, `credentials`, `onboarding` | | Roles/users, regular and temporary files, alarms/conditions and historical access | 74 | `roles`, `users`, `files`, `alarms`, `history` | | Bounded runtime types, XSD/BSD/UA JSON, PubSub and REST OpenAPI artifacts | 6 | `schemas` | | Named PubSub transports, remote configuration, declarative adapters/bridges and SKS | 52 | `pubsub-runtime`, `pubsub-configuration`, `pubsub-bridge`, `pubsub-security` | | Live HTTP(S), MQTT, Modbus TCP and OPC UA WoT forms | 10 | `wot-runtime` | - Reuse typed clients, generated proxies, SDK transports, schema providers and injected services rather than introducing another session manager or protocol engine. - Require host enablement plus exact caller/session/secret/network grants. Protected generic Call/Write/node-management/history/file paths enforce the same policy. - Bind classifications, dispatches, file handles and historical cursors to actual UA generations. Recovery cannot retarget an old handle or authorized mutation. Preserve acknowledged results after late cancellation; do not imply atomicity or retry business commands. - Use opaque secret-store references, explicit writable-store registration, issuer/resource/caller binding and reserved capacity before issuance. Report remote success/local persistence failure as partial completion; redact provider-controlled diagnostic text. - Bound new named runtimes to eight per host, two per caller, five-minute idle expiry and a thirty-minute absolute lease by default. Pending/closing resources consume quota; failed cleanup remains visible. Credential invalidation and session recovery close dependent runtimes. - Keep document planning separate from execution, artifacts root-confined and real JSON inputs typed. Preserve old profile ordinals while widening the set to 64 bits. CI grades fully enabled privileged catalogues rather than connection-only views. **Limitations:** live Kafka remains explicitly unavailable because the backend cannot enforce policy on broker-discovered connections; configuration/schema/capability inspection stays available. The latest public Dekaf v1.23.5 was also checked and its DNS injection remains internal. No private-API workaround or bootstrap-only security claim was added. Full legacy Core registration retains its documented JSON-reflection limitation: native tests isolate its four connection descriptors and validate the expanded production registrations, not whole-host NativeAOT support. ## Related Issues - Fixes #4730. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [x] I have addressed **all** PR feedback received. |
||
|
romanettandClaude Opus 5.5
|
8c52cba0b2 |
CTT reference server: faster alarm cycle in --ctt mode, MinSubscriptionLifetime 1000 ms (#4766)
Shortens CTT runs against the reference server without changing what the CTT verifies. ## Changes - **Alarm simulation step in `--ctt` mode: 1000 → 500 ms.** `AlarmNodeManagerFactory.SimulationInterval` (default stays 1000 ms) is passed to `AlarmNodeManager`; `ConsoleReferenceServer --ctt` sets it via `Quickstarts.Servers.Utils.ConfigureCttModeNodeManagers()` to `Utils.CttAlarmSimulationInterval` (500 ms) before the node managers are created. A full alarm cycle (40 steps through all limits) takes 20 s instead of 40 s. Every alarm still passes through all states and limits; the alarm NodeIds carry the interval (`…ExclusiveLevelAlarmType.500.Alarm` in CTT mode). - **`Ctt.ReferenceServer.Config.xml`: `MinSubscriptionLifetime` 10000 → 1000 ms.** CTT scripts request short subscription lifetimes on purpose and then wait for them to expire (Subscription Basic 063/016, Durable 012, Minimum 02 004, Publish Min 05 Err-001, Diagnostics 024); the 10 s floor inflated those waits. Part 4 sets no minimum; the server still enforces lifetime ≥ 3 × keep-alive. ## Measured (patched CTT 1.05.513 scripts) | Part | Before | After | | --- | ---: | ---: | | A&C (p18), alarm step only + CTT Alarm Cycle Time 20 | 932 s | 576 s | | Subscriptions (p16), MinSubscriptionLifetime only | 679 s | 579 s | Combined full run with the matching CTT-side changes (ReadyToRun server build, publish interval 150 ms, SecureChannel min lifetime 1000 ms, A&C Refresh script waits): **72 → 54 min**, 0 regressions against the CTT baseline (the only error is the known CTT-binary defect GDS 019). The CTT CI repo derives its `Alarm Cycle Time` from `Utils.CttAlarmSimulationInterval`, so runs against branches without this change keep the old 30 s setting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
40288fa7d8 |
Tests: weighted FairRequestQueue test asserts the median of three windows (#4763)
# Description `FairRequestQueueTests.WeightedServiceHoldsUnderConcurrentWorkersAsync` failed once on GitHub Actions macOS net10.0 (#4753, run 38349863236): light=124 heavy=276, ratio 2.23 against an expected 2.5-3.5. It passed on every other CI run and 30/30 locally. **Root cause.** `TrySelect` dequeues an owner's turn token from the lock-free ready queue. On the last token of a turn, `TakeTurnToken` CASes the turn and calls `PushTurn` for the next one. A worker preempted between those two steps leaves the owner with no queued token while it is stalled. The work-conserving dispatch serves the other owner in the meantime, and that share is never made up. Delay injection confirms this: sleeping 1 ms on every 7th turn publish collapses 1:3 to roughly 1:1 (light=199 heavy=201). Publishing a turn ahead of time would only cover stalls shorter than one round (microseconds), while real preemption lasts milliseconds. Making up the lost share (deficit or virtual time) would mean redesigning the scheduler. So this PR documents the bounded skew and makes the test robust instead. **Changes** - `FairRequestQueue` remarks: the weighting holds over time, not in every window, and a preempted worker can cost an owner its share for the length of the stall. - `WeightedServiceHoldsUnderConcurrentWorkersAsync` now measures 3 independent windows through `MeasureWeightedServiceAsync` and asserts that the **median** heavy/light ratio is in 2.5-3.5. The failure message lists all three windows. - Each window uses perOwner = 480 and measured = 560, with queue limits of `2 * perOwner`. That stays under the TestProvider's 1000-request cap and keeps both owners backlogged. - Each window still checks that both owners stay backlogged, then drains the queue and checks that everything is back to zero. The median tolerates one skewed window and still fails when the weighting is lost systematically. With stalls injected into one window only, the median is 3.0 and the test passes (e.g. `light=238 heavy=322; light=140 heavy=420; light=140 heavy=420`). With stalls injected into every window, it still fails. **Validation:** the `FairRequestQueueTests` fixture passes 93/93 on net10.0 Release. The weighted test passed 30/30 runs pinned to 2 cores (`start /affinity 3`). `dotnet format whitespace --verify-no-changes` is clean, and the build has 0 warnings. ## Related Issues - Flaky test seen on #4753 after #4750 (FairRequestQueue without a queue-wide lock) ## 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Marc Schier
|
3e433eb6c4 |
XRegistry: registration lifecycle, identifier allocation, registrar roles and atomic Resource replace (#4756)
# Description Extracts the generic xRegistry registration lifecycle from the WoT work into a standalone, master-targeted change: - Canonical xRegistry identifier allocation (`XRegistryIdentifier`) with case-folded bounded collision handling and exact Version allocation/delete semantics. - Registrar roles / capabilities in the xRegistry NodeSet2 model (only the roles/capabilities facts). - Default-Version views and Resource Meta handling. - FileType read/write modes, session ownership/discard, cursor handling, and short reads until actual EOF on the client. - Atomic whole-document `ReplaceAsync` on the in-memory and file-system resource stores. - Generic projection engine/provisioning and projection contracts, label ownership/no-op behavior, Resource-vs-Version event targets. - Documentation in `docs/XRegistry.md` and XML docs for owned members. - Tests: Identifier, RegistrarRoles, Attribute, Projection, RegistrationEvent, FileMode, ResourceStore, Client, VersionAssignment, concurrent read. Intentionally excluded (handled in separate PRs): trusted Federation / Registry origin model and provider (G9); WoT lease/dispatcher, prepared manifest, 0.7 model/version bump (G12). Merged current `master` ( |
||
|
Marc Schier
|
c2b95e2085 |
XRegistry: trusted logical-Resource federation identity (#4755)
# Description Adds trusted **logical-Resource** federation identity to XRegistry - A federation proxy binds an origin registry (`RegistryOriginDataType`), application, portable `ResourceNodeId` and Xid, never an endpoint or content-digest identity. - `XRegistryFederationTarget`, `IXRegistryFederationProvider`, `XRegistryClient.Federation` (verify/follow/relocate; uses master's `ISessionBindingProvider.CreateBindingAsync`). - `XRegistryFederationNodeManager`, `XRegistryServerOptions`/DI: required target/provider verification and proxy admission. - Model hunks only: `RegistryOriginDataType` 63026 (+encodings 63591/63592), `OriginRegistry` 63593 + `ResourceType` HasProperty, `ExternalReference`/`ResourceUrl` description rewrites. - Tests (`XRegistryFederationNodeManagerTests`), package READMEs, `docs/XRegistry.md` Federation section (links `SessionBindings.md`). Merged current master ( |
||
|
romanettandClaude Opus 5.5
|
0e40e12296 |
CI: CTT regression runs, weekly and on PRs labelled 'ctt' (#4760)
## Summary Adds the **CTT regression** workflow. It runs the OPC UA Compliance Test Tool against the reference server: - every Sunday on master; - manually; - on pull requests a maintainer labels `ctt`. The CTT is member-only software, so the run itself happens in the private repository `OPCFoundation/UA-.NETStandard-CTT`. This workflow does four things: 1. Starts the private run with `workflow_dispatch`, passing the PR head ref and the commit that was labelled. A push after labelling is not run; label again. 2. Waits for the run (about 1.5 hours) and downloads only its public summary: result counts per part and the test cases that changed against the baseline. CTT scripts and logs are never downloaded. 3. Posts the result as the `CTT` commit status, as a PR comment (updated in place) and in the job summary, then removes the label. 4. Feeds a README badge for the weekly master run. The results are a regression signal for this code base, not a certification. The summary says so. ## Security - **Secret access:** `pull_request_target` is used only to get the access secret. The job never checks out or runs PR code; PR data is passed through `env`. - **PR code runs next to the CTT** on the private runner, so it could read the CTT files. Only label a PR after reviewing it. - **Access to the private repository:** a GitHub App installed on that repository only, or a fine-grained token, with Actions read/write. A run takes longer than an App installation token lives (one hour), so the job mints a new installation token for each call. ## Setup needed after merge - [ ] Create a GitHub App, or use an existing one. Permissions: Actions read/write and Metadata read. Install it on `OPCFoundation/UA-.NETStandard-CTT` only. - [ ] Set variable `CTT_APP_ID` and secret `CTT_APP_PRIVATE_KEY` in this repository. Alternatively, set secret `CTT_TOKEN` with a fine-grained token that has the same permissions on the same repository. - [ ] Create the `ctt` label. ## Test plan - [x] Private workflow on GitHub-hosted `windows-latest`. Silent CTT install, server build, generated test PKI and all 23 parts ran in about 83 minutes and matched the local baseline. Remaining differences: Pass→Warning flips from response-timestamp delays, which led to raising Time Tolerance to 250 ms, and one ResendData timing case. - [x] Embedded bash scripts pass `bash -n`; App JWT signing checked against a throwaway RSA key. - [ ] End to end from this repository after merge, once the secret is set: run it manually, then label a PR. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
3a31473cf4 |
Server: no server-wide or per-session locks in per-request session validation (#4753)
# Description
Every request passes through `SessionManager.TryGetSessionContext` at
admission and again at dispatch
(`FairRequestQueue.GetRevalidationStatus` →
`DefaultServerResourceIsolationProvider` →
`ISessionBindingProvider.TryGetSessionContext`), and then through
`Session.ValidateRequest`. Together they took:
1. the server-wide `m_bindingsLock`, to confirm the candidate binding
was still the committed one;
2. the per-session diagnostics lock in `Session.HasExpired`. It is
inlined, so CPU traces charge its spin to `TryGetSessionContext`;
3. the session lock in `Session.IsSecureChannelValid`;
4. the session lock around all of `Session.ValidateRequest`, plus the
diagnostics lock twice (contact refresh, diagnostic counters);
5. the server-wide `SessionManager` event lock, to raise
`SessionDiagnosticsChanged` for every request even when nothing
subscribes.
With the FairRequestQueue lock also removed (a local change without the
queue-wide lock, not yet a PR), these were the next waiting points:
14–16.5 % of on-CPU samples spun in `Lock.TryEnterSlow` under
`TryGetSessionContext`, and once that was fixed another 7–10 % under
`Session.ValidateRequest`.
## Changes
**SessionManager: confirm the binding without the bindings lock**
- `SessionActivationState` publishes a `DispatchBinding` (the committed
binding while no activation is committing, otherwise `null`) with
`Volatile.Write`. The `BindingContext` and `IsCommitting` setters
republish it, and every writer of those already runs under
`m_bindingsLock`.
- The lookup reads it once for the candidate and once to confirm. A
commit always creates a new `SessionBindingContext`, so an unchanged
reference means nothing replaced it. A token is mapped to a session
object at most once and is never re-added after removal, so finding the
token still mapped to the same session afterwards means both held at the
instant of the confirming read, which is the guarantee the lock gave.
- IsCommitting, transfer, close/claim, shutdown and the
certificate/security checks are unchanged.
**Session: read the channel id without the session lock**
- `SecureChannelId` is published with a volatile write in `Activate`,
and `IsSecureChannelValid` reads it without the session lock.
**Session: request validation and diagnostic counters without locks**
- Request counts are interlocked increments of per-session slots (total
and error per `RequestType`, plus unauthorized). They are applied to
`SessionDiagnostics` under the diagnostics lock whenever it is read
(`ReadDiagnostics`, `UpdateDiagnostics`, the diagnostics-node callback,
`ClientLastContactTime`), so readers see the same totals.
- The contact refresh is a compare-and-swap on the tick count, and
requests within the same millisecond do not write. An expiry observed by
`HasExpired` or by a refresh is recorded in the contact word
(`kExpiredFlag`), so a request that read the clock before the monitor
saw the timeout cannot refresh it afterwards. This keeps the documented
"a request cannot revive a session the monitor saw expire" guarantee
without the lock. The intermediate commit that made `HasExpired`
lock-free on its own had opened that window; this commit closes it.
- `ValidateRequest` takes no session lock: the channel id and
`Activated` are published with volatile writes.
- `RaiseSessionEvent` returns before taking the event lock when a
`DiagnosticsChanged` event has no subscriber.
Behaviour differences reviewers should know about:
- `ClientLastContactTime` is millisecond-granular, and two concurrent
refreshes can publish their wall-clock times in reverse order, which
leaves it up to about 1 ms older than the latest contact.
- After an expiry has been observed, an `Activate` no longer refreshes
the contact time. `SessionManager` already rejects activation of an
expired session before that point.
## Tests
- `LookupDuringActivationCommitReturnsNoBindingAsync`: a lookup inside
the commit window (after `Session.Activate`, before the binding commit)
returns no binding. A test session re-implements `ISession.Activate` to
run the hook. Before this test, publishing the binding regardless of
`IsCommitting` passed all 690 session tests.
- `LookupRacingTransfersAndCloseReturnsOnlyCommittedBindingsAsync`: 4
readers look up while 1000 transfers alternate between two channels,
followed by a close. Each result must be a committed binding of the
queried channel, no older than the last completed activation and never
decreasing, and nothing may come back after the close.
- `RequestRacingObservedExpiryDoesNotReviveTheSessionAsync`: a clock
hook lets the monitor observe the expiry between a request's clock read
and its refresh, and the request must fail with `Bad_SessionClosed`. It
fails if `HasExpired` does not record the expiry.
- `ConcurrentRequestsAreCountedExactlyAsync`: 8 threads × 2100
validations (Read/Browse, plus rejected Writes). The totals, per-service
counts, error counts and unauthorized count must be exact, and a
concurrent reader never sees the total go backwards.
- Local runs (net10.0): `Opc.Ua.Server.Tests` 8487 passed / 0 failed;
`Opc.Ua.Sessions.Tests` 851 passed / 0 failed / 379 skipped. net48
builds.
## Measurements
`perf/ServerLoadHarness`, Server GC, paired alternating A/B, 7 rounds.
Baseline is master
|
||
|
romanettandClaude Opus 5.5
|
a14b23e7dd |
Server: cheaper symmetric crypto path, FairRequestQueue without a queue-wide lock (#4750)
# Server: cheaper symmetric crypto path, FairRequestQueue without a queue-wide lock Roadmap items E1 and 5b of the o6 benchmark gap analysis, one commit each. - **E1:** Windows one-shot `Aes.EncryptCbc`/`DecryptCbc` import the key (`BCryptImportKey`) on every message, although #4720 caches one `Aes` per channel token. The AES-GCM and ChaCha20-Poly1305 policies also created a new keyed instance per message. - **5b:** with `AsyncCustomNodeManager`, which is what the o6 benchmark's 2.0.1 worker uses, the `FairRequestQueue` gate became the largest waiting point. `TryEnqueue`, `TrySelect` and `TryDequeue` spun on one `Lock` for every request. No wire, configuration or public API changes. ## Results These are paired A/B runs against `origin/master`, five rounds per scenario. The two harness builds alternate per round, starting side swapped every round, on the same shared machine (Ryzen AI 9 HX 370, 12 cores / 24 threads, Windows 11). The tables show medians, with ranges in brackets where they matter. The server is configured like the o6 2.0.1 worker: `AsyncCustomNodeManager`, Server GC, queue sized clients × outstanding + 3. o6read uses the benchmark's native open62541 client workers; `read` uses the harness's .NET client. On this machine throughput moves between rounds by up to 2×, so CPU and bytes per request are the reliable signals. ### E1: symmetric crypto (measured on |
||
|
romanettandClaude Opus 5.5
|
63adef0d10 |
Fix the Sessions LoadTest fixture and the server bottlenecks it exposed (#4749)
The `[Explicit]` Sessions `LoadTest` fixture has not started since #4541 (also on the 2.0.0 tag): it set `MaxChannelCount = MaxSessionCount = 10100`, and resource isolation refuses to start unless the channel cap covers the sessions plus the bootstrap and reconnect floors plus one. Every LoadTest case failed in `OneTimeSetUp`. Running the fixture again, and profiling it and the `perf/ServerLoadHarness`, found several bottlenecks that shipped in 2.0. ## Changes **Test** - `MaxChannelCount = MaxSessionCount + 3`, the same margin as the framework defaults (100 / 103). - Notifications are counted with per-node `Interlocked` counters. The shared `ConcurrentDictionary<NodeId, int>`, updated by 750 subscriptions, took 13 % of the process CPU in bucket-lock contention. - After the writer stops, and after the last event, the tests wait until the received count stops growing instead of a fixed delay, and log the drain time. **Server / client** - **Detached monitored item recovery** (#4094): every node a NodeManager added (`AddPredefinedNodeAsync`, recursively for children) scanned every monitored item of every subscription for detached items. `CreateSubscription` adds ~30 diagnostics nodes, so with 450k monitored items subscription creation was quadratic: 69 % of CPU, and `CreateSubscription` timed out. `MonitoredItem` now keeps an exact process-wide count of live detached or deleted items, and `MasterNodeManager` skips recovery while it is zero. All flag changes go through one helper under the item lock; a disposed item does not count, because it cannot be attached again. - **Client channel gate** (#4232): `UaSCUaBinaryClientChannel.SendRequestAsync` entered the gate with a blocking `Gate.Enter()` inside an async method, and `SemaphoreSlim.Wait` spun for 19 % of CPU with many requests in flight on one channel. It now awaits `Gate.EnterAsync(ct)`, like the server side. - **ResourceManager** - Status code and symbolic id translation looked up their maps under the manager lock and translated while holding it. Sessions that request diagnostics do this for every notification: 83 % of lock contention. The maps are copy-on-write and read without the lock. - Translation tables were `SortedDictionary<string, string>` with the default, culture-sensitive comparer: up to 1 µs per Browse display name, 22 % of Browse server CPU. They now use an ordinal `Dictionary`. - **Namespace default permissions**: every permission check read `DefaultAccessRestrictions` / `DefaultRolePermissions` / `DefaultUserRolePermissions` of the node's namespace through two server-wide locks and deep-copied the role permission array, per node. That was 99 % of lock contention for Read/Write of 100 values. The values are now read once per namespace per service request and remembered on the `OperationContext`. Contexts that are not a service request (monitored item and session contexts, which can live long) remember nothing, so a changed default applies to them at once. ## Results `perf/ServerLoadHarness`, paired A/B against `master` (3 alternating rounds, medians, 20 sessions, 24 cores). The harness numbers predate the recovery, client gate and status code translation commits, which those scenarios do not exercise: | Scenario | Throughput | Server CPU / op | Server alloc / op | Lock contention | |---|---|---|---|---| | Read, 100 values | +19 % | −28 % | −18 % | −96 % | | Read, 100 values, SignAndEncrypt | +27 % | −26 % | −18 % | −98 % | | Write, 100 values | noise ¹ | −19 % | −17 % | −88 % | | Browse (Server object) | +3 % | −16 % | −10 % | −2 % ² | | Read, 1 value | +12 % | −4 % | ±0 | ±0 | ¹ Throughput varied run to run on the shared machine; per-op CPU and allocation are the reliable signal. ² Browse contention is the Server node's browse lock in `NodeState.CreateBrowser` (all sessions browse the same node); not changed here. `LoadTest` (opc.tcp, Release, in-process client and server): | Case | master | this PR | |---|---|---| | `ServerReadLoadTestAsync` | setup failure | passes (50M value reads, 0 errors) | | `ServerManySessionsLoadTestAsync(500)` | setup failure | passes | | `ServerSubscribeLoadTest_Classic` | setup failure; on 2026-09-26 (before #4541): `CreateSubscription` `BadRequestTimeout` | 76–85 % received, **fails** (needs 99 %) | | `ServerSubscribeLoadTest_ManagedSessionV2` | setup failure; 2026-09-26: 1,885 notifications received | 35–45 M notifications, 77–85 % received, **fails** | | `ServerEventSubscribeLoadTestAsync` | setup failure; 2026-09-26: 34.6 % | passes: 100 % delivered, drained ~40 s after the last event | The subscribe cases now lose notifications rather than deliver them late: delivery stops ~2 s after the last write. Every monitored item has queue size 1 and the writer is not paced by delivery, so whatever exceeds the publish rate (~585k notifications/s here, SignAndEncrypt with the client in the same process) is overwritten. The event test lost nothing either: it waited a fixed 2 s after the last event and counted the backlog as lost. It now waits until delivery stops. Its throughput is bound by garbage collection: the event consumer of the Server node spends ~90 % of its wall time suspended for workstation GCs of the shared test process. With `DOTNET_gcServer=1` it generates 702 instead of ~210 events/s and drains in 7 s instead of ~40 s, and the subscribe cases reach 87 % (Classic) and 93 % (ManagedSessionV2) at 78–106 M notifications. ## Testing - `Opc.Ua.Server.Tests` 8468 passed / 5 skipped, `Opc.Ua.Client.Tests` 2788 passed, `Opc.Ua.Sessions.Tests` 851 passed / 379 skipped, `Opc.Ua.Core.Tests` Tcp/Channel 797 passed (net10.0). - New `OperationContextDefaultPermissionsCacheTests`. - `Opc.Ua.Server` builds for net48; `dotnet format whitespace` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Marc Schier
|
7c965fd636 |
Capture metadata images and generation-bound session clients (#4752)
# Description - Add isolated NodeState reference images, TypeTable graph snapshots, EncodeableFactory registration forks/views, StringTable mutation versions/private copies, and ServiceMessageContext mapping-holder generation tracking. Reference images are lazy for empty nodes; selected-image clones own independent storage and publication ownership. - Add the complete generic NodeBrowser continuation-dependency capability. This does not implement server browse-history retention, NodeManager read images, or server batch/readiness admission. - Add optional client binding capture for SessionClient, native Session and ManagedSession, with managed-lease/native-channel generation composition. Bindings capture authenticated-session identity, mapping holders/versions and owned endpoint/policy/configuration observations. Replacement, reconnect, mapping ABA and disposal permanently invalidate stale bindings; disposing a binding does not dispose its owner. - Preserve cancellation and master pooled-response ownership: a rejected pooled response is released exactly once; an accepted response remains caller-owned. Keep native transport submission under its replacement gate and completion outside the gate, without routing ordinary unbound requests through binding machinery. - Rename the existing server lookup interface and all actual consumers with minimal hunks; add focused fixtures, generated FileTypeClient usage guidance, and package/transport documentation. **Breaking preview type identity:** the existing 2.0-preview server lookup `Opc.Ua.ISessionBindingProvider` becomes `Opc.Ua.IServerSessionBindingProvider`. Server lookup consumers must update their referenced type and rebuild. This is neither source-compatible nor binary-additive for that preview type. The new client capability is optional and adds no mandatory members to existing ISession/ISessionClient/ITransportChannel contracts. This preview-to-preview rename is documented in SessionBindings/Transports, not in the 1.5-to-2.0 MigrationGuide. **Ownership boundary:** no RequestLifetime changes, certificate-manager/channel-entry adoption, BaseVariableState changes, Method-result feature extraction, NodeManager batch/readiness or browse-history retention, session restoration, registry federation, WoT conversion, or WoT runtime/planners/events. DependencyInjection/ResourceIsolation/index changes are lookup/link closure only. SessionManager's PR hunk changes only its implemented lookup-interface name; newer master public diagnostic masking is not replaced. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Part of #4511: user-approved combined groups 5+6 Wave A extraction. This PR does not close the umbrella or publish Wave B. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [ ] 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. - [x] I have addressed **all** PR feedback received. ### Review qualification Review fixes are published at **`6b0eb1d5c32a439300fbcaff974b9af8b909161b`**, changing eight already-owned files. All five received review threads were replied to and resolved after verification. Factory selector installation and target-selection-plus-commit share the root publication gate, with consistent root→image release/capture ordering. TypeTable selector installation and final mutation routing share its existing gate. Native bound channel validity uses volatile observations; the two owned XML defects are corrected. | Requirement | Evidence | |---|---| | Selector installation must not hide TypeTable mutations | `SelectorInstalledDuringNodePreparationReceivesMutation`: old Add routing failed on both TFMs; corrected routing passes on both within **148/148** factory/TypeTable controls | | Selector installation must not race factory commit routing | Static common-root-gate/order proof plus baseline-GREEN `BuilderCapturedBeforeOwnershipUsesSelectedFactory`, `ViewOwnerReleasePreservesRegistrationsAndAllowsReuse`, `ViewOwnerReleaseCanRetryAfterPendingPublication`, `PublicationProtectsPendingAndRetiredFactoryImages` | | Lock-free validity must observe owner transitions | Static volatile-read reasoning plus native `BoundRequestCannotDispatchWhileNativeReplacementIsPendingAsync`, `BoundResponseRejectsInvalidationAfterNativeDispatchAsync`, `BoundClientCannotRetryAcrossFaultedManagedEntryReplacementAsync`: **12/12 each TFM**; Core binding/pooled ownership controls **17/17 each TFM** | | Correct owned XML documentation | “an unmanaged” and one multiline `ReferenceUpdate.Next` summary; scoped whitespace/INFO and build evidence below | Factory controls are **not** claimed as a deterministic factory RED, and public transport controls are **not** claimed as a deterministic weak-memory RED. No private-lock reflection, artificial production seam, or source-name assertion was added. The TypeTable RED uses the real `ILocalNode.NodeClass` callback during node preparation, and both expected RED receipts/TRX were retained. All accepted review runs use the unchanged admitted SDK wrapper, normal project references, sole mutex, one node/no shared compiler, finite resource guards and canonical short-C build outputs. Source hashes were frozen per command and matched the committed bytes afterward. SDK ownership was released **before** commit/push/review replies. No automatic merge was requested. Scoped whitespace passed across the four affected projects. Review INFO classification is **0 owned findings / 13 untouched baseline findings excluded** (whole-file formatter exits 1,1,0,0); no baseline RCS1140/RCS0055 expansion or suppression. The final normal net10 public Client/Core/Types trim/AOT-analyzer closure passed **0 warnings / 0 errors**. This is not a NativeAOT publish/run. <details> <summary>Accepted review command payloads and results</summary> These exact argument payloads were executed through the unchanged guarded SDK wrapper. `$proof` expands to the protected results-directory below. Successful explicit builds of the same source preceded the `--no-build` runs. ```powershell $proof = 'C:\Users\mschier\.copilot\session-state\a3b74048-c52f-4c87-8773-7a46c9ab0087\files\qualification-review-4752' # Exit 0: 148 passed, 0 failed dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -f net48 -p:CustomTestTarget=net48 -p:UseArtifactsOutput=true -p:ArtifactsPath=C:\cp56-a3b7-v2 -nr:false --no-restore --verbosity minimal --filter "FullyQualifiedName~EncodeableFactorySnapshotTests|FullyQualifiedName~TypeTableTests" --logger "trx;LogFileName=review-types-green-net48.trx" --results-directory $proof # Exit 0: 148 passed, 0 failed dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -f net10.0 --no-restore --no-build --filter "FullyQualifiedName~EncodeableFactorySnapshotTests|FullyQualifiedName~TypeTableTests" --logger "trx;LogFileName=review-types-green-net10.trx" --results-directory $proof -p:CustomTestTarget=net10.0 -p:UseArtifactsOutput=true -p:ArtifactsPath=C:\cp56-a3b7-v2 -nr:false --verbosity minimal # The following two per-TFM payloads each exited 0: # Core: 17/17 net48 and 17/17 net10.0; Client: 12/12 net48 and 12/12 net10.0 foreach ($tfm in @('net48', 'net10.0')) { dotnet test tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -f $tfm --no-restore --no-build --filter "FullyQualifiedName~SessionBindingResponseTests|FullyQualifiedName~SessionBindingContextTests" --logger "trx;LogFileName=review-actual-core-green-$tfm.trx" --results-directory $proof -p:CustomTestTarget=$tfm -p:UseArtifactsOutput=true -p:ArtifactsPath=C:\cp56-a3b7-v2 -nr:false --verbosity minimal dotnet test tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj -f $tfm --no-restore --no-build --filter "FullyQualifiedName~BoundRequestCannotDispatchWhileNativeReplacementIsPendingAsync|FullyQualifiedName~BoundResponseRejectsInvalidationAfterNativeDispatchAsync|FullyQualifiedName~BoundClientCannotRetryAcrossFaultedManagedEntryReplacementAsync" --logger "trx;LogFileName=review-actual-client-green-$tfm.trx" --results-directory $proof -p:CustomTestTarget=$tfm -p:UseArtifactsOutput=true -p:ArtifactsPath=C:\cp56-a3b7-v2 -nr:false --verbosity minimal } # Exit 0: 0 warnings, 0 errors dotnet build src\Opc.Ua.Client\Opc.Ua.Client.csproj -f net10.0 -p:CustomTestTarget=net10.0 -p:UseArtifactsOutput=true -p:ArtifactsPath=C:\cp56-a3b7-v2 -nr:false --no-restore --verbosity minimal ``` The six expanded GREEN commands, exact results/TRX hashes and two expected RED commands are retained in `qualification-review-4752\qualification-summary.json`, SHA256 `C5E71A4D8D37BAC8803F6DF38B6BCE2B70F3D0AFD5DEC8493FAE9DB440DCFB5A`. Committed/qualified byte identity is in `publication-identity.json`, SHA256 `CD9515FE96703F8B241F2B7B9B71F73EF0FF2626C2C8BCCE40307971A8F8E809`. Per-command raw exits, guards, hashes and reports are retained outside disposable build roots. For each record in the protected `format-inputs.json` (Types/Core/Client libraries and Types.Tests), the recorded Project/Include values expand these exact payloads: ```powershell # Workspace properties recorded with the formatter receipts: $env:CustomTestTarget = 'net10.0' $env:UseArtifactsOutput = 'true' $env:ArtifactsPath = 'C:\cp56-a3b7-v2' # All four exit 0 dotnet format whitespace $Project --no-restore --verify-no-changes --verbosity minimal --include $Include # Exits 1,1,0,0 due only to untouched baseline findings; owned=0, baseline=13 dotnet format $Project --no-restore --severity info --verify-no-changes --verbosity minimal --report "$proof\formats\review-info-$i" --include $Include ``` Initial fresh matrix evidence remains separately identified and unchanged: Types274/Core119/Client151/server137 net10; Types274/Core119/Client151/server135 net48, all passing. Qualification base remains `0dff94bd378e79b8cf1a47e1aaf340f9b4b40048`; the initial published head was `9fee4583`. All 16 initial exact command arrays/results/TRX hashes remain in protected `qualification-v2\qualification-summary.json`, SHA256 `88E10240C20049B59A3401BE007C6528A85DF2F3DCFAF9D11D908CEF2946DA4E`. Failures/incidents remain retained rather than relabeled as success: original disappeared-root historical counts are invalidated; the large-TRX controller DOM-parser failure is preserved separately from accepted streaming receipts; the review argument-array defect/default-D-output controls are preserved but excluded from canonical review qualification. Correctly separated argument rechecks produced the accepted short-C Core/Client receipts. The sole SDK wrapper was not changed. Full UA.slnx, measured coverage increase, CLA verification, NativeAOT publication and current-head/master CI/CodeQL remain unclaimed. Actual merge—not resolved review threads—unlocks Wave B. </details> |
||
|
Marc Schier
|
84d6387a4f |
Make request cancellation own exactly one terminal state (#4744)
# Description `RequestLifetime` now publishes the winning cancellation status and terminal ownership in a single atomic state, so concurrent client Cancel, Session close and timeout can never overwrite each other's status or double-own the terminal transition. External token registrations are owned by the lifetime and released on completion, and disposal is deferred until running cancellation callbacks have finished (no use-after-dispose of the CancellationTokenSource). Adds regression tests: 16384-iteration race tests asserting status/owner/callback outcomes with a finite 2 min watchdog (`RequestLifetimeTests`, `RequestManagerTests`) and `WaitAsync` cancellation tests (`TaskWaitAsyncTests`; the polyfill change is only 2 XML doc lines, no runtime change). Files: `RequestLifetime.cs`, `System.Threading.Tasks.cs` (docs), and the three test files. `StandardServer.cs` is intentionally untouched (already on master). ## Related Issues - None. ## Checklist - [ ] 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. - [ ] 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. Validation actually run (net10.0 and net48, all passed): Types.Tests `TaskWaitAsync` 23/23; Core.Tests `RequestLifetime` 16/16; Server.Tests `RequestManager` 56/56; `dotnet format whitespace --verify-no-changes` on owned files. Full UA.slnx not run. |
||
|
Marc Schier
|
184f19a2b0 |
Add complete method invocation results and resolved diagnostics (#4747)
# Description `MethodInvocationResult` carries the operation result, declaration-ordered outputs and optional position-preserving input results. Asynchronous callbacks retain the actual receiver, cancellation token and resolved diagnostic strings. Fluent predefined, typed, instance, reference-child, resolved and virtual method builders reject conflicting handlers in either registration order and never overwrite materialized handlers. Direct configuration remains available. Shared Call-result assembly retains input diagnostics, including useful text on Good input positions, and requested operation diagnostics on Good results. Diagnostic encoding preserves nested Good-status text, status information bits and the outer-inclusive depth limit. Request admission clears client-supplied internal diagnostic permission flags before early returns or validation failures; admitted sessions grant the flag only after the existing administrator-role checks. Includes corresponding unit/integration regressions, complete XML API documentation, and method-result guidance consolidated with the fluent APIs in `docs/NodeManagers.md`. No WoT receiver/projection/remote-execution, event-readiness/native-admission, or bulk/history implementation is included. ## Related Issues Part of the user-approved two-wave extraction plan tracked by #4511. _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Related to #4511. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [ ] 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. - [x] I have addressed **all** PR feedback received. |
||
|
romanettandClaude Opus 5.5
|
8b360da7d2 |
Server publish path: no deep clone per Publish, pooled notifications that are actually reused, ready list instead of an O(N) scan (#4748)
Roadmap item A5 of the o6 benchmark gap analysis. Every Publish response deep-cloned its NotificationMessage in `SentMessageQueue` (PR #4707 kept the clone because a retained message returns its pooled payload when the client acknowledges it, which can happen before the response was sent). This PR replaces the clone with a send-completion lease, makes the activator pool that recycles the notifications O(1) and sizes it for the server's load, builds messages without intermediate copies (the notification array is rented), and lets monitored items tell their subscription when they have something to report so the publish timer no longer walks every item. ## Results Paired A/B against master `0dff94bd3` (includes #4742 and #4743; alternating builds, 3 rounds, medians; 20 sessions unless noted, workstation GC, 24-core machine shared with other sessions). Server = the reference server child process. | Scenario | Server alloc / op | Server GC pause | Server CPU / op | Write→notify p50 / p99 | Throughput | |---|---|---|---|---|---| | `sub`, 5 subs × 100 items, pub 100 ms | **−57 %** (373 → 159 B) | **−57 %** | −7 % ¹ | −1 % / −2 % | +1 % ² | | `sub`, 5 subs × 100 items, pub 10 ms | **−57 %** (375 → 163 B) | **−52 %** | +9 % ¹ | −22 % / +1 % | ±0 ² | | `sub`, pub 100 ms, SignAndEncrypt | **−57 %** (380 → 163 B) | **−53 %** | +4 % ¹ | −11 % / **−12 %** | ±0 ² | | `sub`, 2 sessions × 1 sub × 1000 items, 20 changing, pub 50 ms (9 rounds) | −2 % ³ | ±0 | −23 % ³ | ±0 / −2 % | ±0 | | `counters`, 4000 items, 10 ms sampling, pub 1 s (per notification) | −2 % | −6 % | −6 % | – | ±0 | | `o6counters`, 2048 items (per sample) | −3 % ⁴ (111 → 108 B) | −5 % | −7 % | – | ±0, all PASS | | `o6counters`, 4096 items (per sample) | −3 % ⁴ | +14 % ⁵ | −6 % | – | ±0, all PASS | ¹ CPU per notification varies by ±15 % between rounds on this machine; in an earlier 8-round batch against `071660354` the same scenarios measured −14 % / −1 %. Read it as "no regression". Client CPU per notification moved −18 %…0 %, within its spread. ² Bounded by the writer rate. ³ The server is nearly idle (0.05–0.12 cores, 360 notifications/s; its allocation is the writer and keep-alives). The first 3 rounds measured +31 % CPU, 6 more rounds −23 % (medians 969 → 742 ms per window): noise-bound, no regression. ⁴ With #4743's sampling permission cache a sample costs 111 B; one Publish per second carries 2048/4096 notifications. ⁵ The activator pools no longer allocate their 1023-slot arrays up front; the heap is a few MB smaller, which shifts the GC's generation budgets so that some low-allocation runs make every collection gen1 (26 of 26 instead of 2). Pause totals differ by milliseconds per 10 s window (37 → 42 ms here, 21 → 20 ms at 2048 items). Allocating the arrays eagerly restored master's gen1 counts in a separate test. In a single CPU trace of `sub` at pub 10 ms, recycling acknowledged payloads (pool `Return`) went from 16 % of server on-CPU time (master, the full pool scanned per return) to 5 %. ## Changes - **Send completion instead of a deep clone** (`NotificationMessageLease`, `LeasedPublishResponse`, `SentMessageQueue`): the retransmission queue and every Publish response that carries a message each own a reference. The transports already call `IPooledEncodeable.Reuse()` on a response once it is encoded or dropped; the Publish response is now a `LeasedPublishResponse` whose `Reuse()` releases its reference. The pooled payload (`NotificationMessage`, `DataChangeNotification`/`EventNotificationList`, `MonitoredItemNotification`/`EventFieldList`) is recycled when the last owner releases it, so an acknowledgement that overtakes the send no longer recycles a payload that is still being encoded, and the queue and the response share one message. A reference that is never released only leaves the payload to the GC; every owner releases at most once. Keep-alive messages are owned by their response alone. Republish, the durable snapshot and the retransmission mirror still copy. - **Transports**: the HTTPS (binary and JSON), WebSocket (UA-SC, JSON, OpenAPI) and Kestrel TCP listeners and the WebAPI endpoint dispatcher now release responses after encoding, as the TCP listener does, so leased payloads are recycled there too. A response retained for a reconnect is released when it is sent; a send that throws leaves the response to the GC. - **Activator pool** (`BoundedObjectPool`): rent and return scanned the 1023 slots linearly; with the pool full every returned notification scanned all of them before being dropped (16 % of server CPU at 94k notifications/s). It is now a `ConcurrentQueue` behind the lock-free first slot with an `Interlocked` bound (a locked stack spun under contention when transport threads return whole messages). The queue is created on first use, so activators of types that are never recycled no longer allocate a 1023-slot array each. The client decoder rents through the same pool. - **Pool bound** (`PooledEncodeableType<T>.EnsureMaximumRetained`, internal): the bound can be raised, never lowered. The subscriptions raise the `MonitoredItemNotification` and `EventFieldList` pools to twice the number of monitored items in the process (a notification is recycled about two publishing cycles after it was built), between the default 1024 and 65536. At the cap the retained notifications take about 5 MB; the bound stays raised after items are deleted. - **Message construction** (`Subscription.InnerPublish`/`ConstructMessage`, `MonitoredItem.Publish`): the per-Publish `Queue<>`/`List<>` objects are reused by the subscription; messages are built at their final size (no `List`→`ArrayOf` copy, no `AddItem` reallocation); the `MonitoredItemNotification[]` of a message is rented from `ArrayPool` and the message carries an exact-length slice of it; the lease returns the array (cleared) with the payload, and messages dropped on queue overflow return theirs at once; only arrays the subscription rented are tracked, so restored, mirrored and copied messages never reach the array pool; the `DiagnosticInfo` list is only allocated when a notification has one; timestamps are stripped on the struct before the pooled notification is filled. - **Ready list** (`IReadySignalingMonitoredItem`, `Subscription.PromoteReadyMonitoredItems`): `MonitoredItem` acquires a ready signal (compare-exchange, a full fence after its flags are set) when it queues a value or event or a resend is requested, and hands itself to the subscription's intake list. The publish timer checks only these candidates instead of every item under the subscription lock. An item stays a candidate while it has something pending that can become reportable without a new value (sampling deadline, aggregate interval, samples of a SAMPLING item, trigger flag); after releasing the signal the subscription checks again, so a value queued concurrently is never missed. Created, restored, modified, monitoring-mode and triggering changes mark candidates, as does an item that leaves the publish list with something pending. ### Behaviour notes (custom subscription or monitored-item extensions) - **Publish responses share the retained message.** `PublishResponse.NotificationMessage` returned by `SubscriptionManager.PublishAsync` is the retained message itself, not a copy, and its payload objects return to the activator pools after the response was sent and acknowledged. Code that keeps or modifies the message of a Publish response (for example in a `StandardServer.PublishAsync` override or a request interceptor) must copy it first (`CoreUtils.Clone`). `Republish` still returns a copy. - **Custom transports**: a transport that does not call `Reuse()` on responses keeps working; the payload is then collected by the GC instead of being pooled. - **Custom `IMonitoredItem` implementations** (not derived from `MonitoredItem`) cannot signal readiness: a subscription that contains one scans all its items on every publishing cycle, as before. `MonitoredItem` subclasses signal through the base class. - **`DataChangeNotification.MonitoredItems` of a published message is a slice of a pooled array**: read it through `ArrayOf<T>` (`Span`, indexer, `Count`); code that unwraps the underlying array would see a longer array. - **Order within a message**: notifications of different monitored items appear in the order the items became ready rather than in the internal list order. OPC UA does not define an order across items; the order of the values of one item is unchanged. ## Harness (`perf/`) - `--write-nodes K` for `sub`: the writer changes only the first K monitored nodes, for large subscriptions in which few items change. - `server-scenarios.txt`: publish-path scenarios (`--pub 10`, 1000 items per subscription with 20 changing). ## Testing - New `NotificationEncodingLifetimeRegressionTests` cases: an acknowledgement before send completion does not recycle the payload still being encoded (the send completion does); Republish after the send completed returns the original content and stays independent of the recycled original; a failed send that releases the response twice, plus a copy of the response, recycles the payload exactly once and only after the queue released it; a keep-alive is owned by its response; the rented notification array stays intact until both acknowledgement and send completion (either order), the message encodes only its slice, and queue overflow returns the arrays of dropped messages. The existing 30-case matrix (publish / queued / republish / mirrors × acknowledge / evict / clear × data / events) now asserts that a shared payload survives every queue mutation until its response is released. - New `ReadyListRegressionTests`: the same scenarios on a subscription that uses the ready list and on one that scans (it also has an item that cannot signal) must publish the same notifications, for triggering (sampling items, backlog of the triggering item), monitoring-mode changes (Sampling↔Reporting, Disabled, disabled before publish), disabled publishing with keep-alives and ResendData, a value queued while the item was in the publish list, and the backlog of an item that left Reporting while in the publish list. Mutation check: removing the item's signal fails 5 of 6, removing the subscription-side marks fails the backlog case. Plus: a published item without pending values is not tracked; creating 1200 items raises the notification pool bounds to at least 2400. - `PooledEncodeableTypeTests`: the raised bound is respected and never lowered. - net10.0 on the rebased branch: Server 8481 passed (5 skipped); Sessions 851 (379 skipped); Client 2788; Subscriptions.Classic 57; Subscriptions.Durable 36 (11 skipped); Redundancy.Server 884, Redundancy.Client 134, Redundancy.Samples 17; Stress Contract+Integration 45; Subscriptions 657 (6 skipped); Types pool tests 165. All 0 failed. - net48 build of `Opc.Ua.Server.Tests` and `Opc.Ua.Bindings.Https` and `Opc.Ua.Types.Tests` clean; `dotnet format whitespace` clean. ## Not changed (follow-ups) - Republish still copies the message (`ISubscriptionManager.Republish` returns a `NotificationMessage`, so the response cannot carry a lease without an API change). - `EventFieldList[]` per event message could be rented like the data change array. - Sampling remains the larger share of allocation and CPU in the `counters`/`o6counters` scenarios. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Marc Schier
|
39dd5632e3 |
Fix cache invalidation after accepted async indexed writes (#4746)
# Description After the full async write hook accepts an indexed update, merge the slice into the whole cached value when possible. If the cache is missing or stale and cannot absorb the slice, clear the cached value, set its status to `BadWaitingForInitialData`, and reset its timestamp to `DateTimeUtc.MinValue`. Preserve the handler's successful operation result, including `GoodClamped`, and report a value change. Never expose the replacement slice or stale data as the whole successfully updated cache. Synchronous fallback and simple-hook behavior are unchanged. The existing fixture remains intact, with its unusable-cache expectations updated for this intentional behavior correction. Fourteen additional cases cover successful merges, missing/stale cache invalidation, Good/GoodClamped operation results independently of cached status, handler rejection, type/rank validation, node/user access, and unsupported status/source/server timestamps. XML documentation explains the cache-versus-operation contract. Scoped analyzer cleanup uses asynchronous fixture helpers without removing the cancellation, lock-release, access, type, status, timestamp, or change-mask oracles. **Validation:** final head `aaf78b8df1bed46fb14033f3f3c9f52945469000` passed 32/32 cases on actual net10.0 and 32/32 on actual .NET Framework 4.8, with zero failures or skips. Both owned fixtures and the owned production file passed formatting/analyzer verification at info severity. The normal-reference .NET Standard 2.0 Types build passed with zero warnings/errors. The full `UA.slnx` suite and numerical coverage collection were not run locally; no full-suite or coverage-percentage claim is made. ## Related Issues Related to [#4511](https://github.com/OPCFoundation/UA-.NETStandard/pull/4511), the approved Wave A extraction plan. This standalone master PR does not close the tracking PR or imply completion of the remaining extraction groups. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [ ] 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. |
||
|
Marc Schier
|
4fd4535362 |
Fix managed-channel certificate ownership on early create failure (#4745)
# Description `CreateChannelAsync` now releases transferred certificate/chain handles when disposal or argument validation fails before transport creation. Managed entries use the existing inherited channel-creation method directly and share entry-retained, idempotently disposed handles with the transport, without adding another reference for non-adopting transports. The ownership contract is documented on the existing method; no internal redeclaration or forwarding implementation is needed. New and changed ownership APIs have multiline XML summaries and parameter documentation. Fourteen regression cases use generated certificates persisted in an isolated directory store and resolved through the real `CertificateManager.CertificateProvider`. They sign and verify SHA-256 digests to prove private-key usability through failed open/retry, publication/rotation, forward/reverse connection, and shutdown, and probe each certificate core's final retirement independently of global counters. Both adopting and non-adopting transports and optional chains are covered. Temporary stores are cleaned up. ## Related Issues Related to #4511: user-approved independent Wave A group 2 extraction. This PR does not close the overall tracking issue or include the other extraction groups. _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [ ] 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. Tests were added and verified as described above; the coverage checklist remains unchecked because coverage was not measured. |
||
|
romanettandClaude Opus 5.5
|
0dff94bd37 |
Server quick wins from the o6 benchmark gap analysis: admission sizing, sampling permission cache, routed Read dispatch (#4743)
The o6-automation benchmark gap analysis listed several server quick
wins that need no wire or public API change. This PR implements three of
them. The 2.0 server now passes the benchmark's 10 clients × 1000
outstanding Reads test with the benchmark's own queue sizing. Its 100 Hz
sampling capacity grows about 7–10× on the same machine, and a
single-value Read allocates 8 % less.
## Results
Paired A/B against `master` (
|
||
|
romanettandClaude Opus 5.5
|
25ebf5a4d8 |
Server: remove the server-wide locks from the request pipeline, bounded transport buffer pool (#4742)
# Server: remove the server-wide locks from the request pipeline,
bounded transport buffer pool
Quick wins #5 and #6 of the o6 benchmark gap analysis. Under 10 channels
× 100 outstanding single-value Reads on 24 cores, about 59 % of the
server's on-CPU time spun in `Monitor`/`Lock` slow paths, and roughly 20
of the ~30 lock acquisitions per Read were on server-wide locks in the
request pipeline. Separately, on hosts with one or two processors every
response allocated a fresh 64 KiB chunk, because
`ArrayPool<byte>.Shared` runs out with about a thousand responses in
flight.
No wire or configuration changes. The only public API change is an
additive `RequestLifetime` constructor. The admission *sizing*
(`MaxQueuedRequestCount` → shared capacity and reserved floors) is
untouched; this PR only changes how the accounting is locked.
## Results
Paired A/B against `origin/master` (
|
||
|
Marc Schier
|
1bb4bdca1d |
Add Machine Vision Parts 1 and 2 (#4733)
# Description Implement the released [OPC 40100-1](https://reference.opcfoundation.org/specs/OPC-40100-1) and [OPC 40100-2](https://reference.opcfoundation.org/specs/OPC-40100-2) specifications with generated models, provider-backed runtimes, typed clients, MCP workflows, and DI/Machinery integration. - **Part 1 stays in `Opc.Ua.Vision`** and its client/server packages alongside the distinct Perception model. Provide control/acquisition state machines, jobs and synchronized steps, recipe/product/configuration/result management, file transfers, diagnostics, events, and conditions. - **Part 2 is in `Opc.Ua.MachineVision`, `.Client`, and `.Server`.** Provide asset categories, identification, image-handling aspects, health/maintenance data, topology, and transactional updates. The base Vision assemblies do not reference these optional packages; the Part 2 wire model does not depend on Part 1. - Preserve the release-pinned model bytes, wire namespaces and NodeIds. Perception types are in `Opc.Ua.Vision.Perception`; the AMCM generated namespace remains `Opc.Ua.Vision.MachineVision.AMCM`, and asset client/server extension namespaces remain `Opc.Ua.Vision.Client` and `Opc.Ua.Vision.Server`. - Use `AddOpcUaMachineVision` and `AddOpcUaMachineVisionAMCM` for model loading and factory registration. Each has node-collection, encodeable-factory-builder and node-state-factory-builder overloads. These short entry points delegate to the generated implementations; existing namespace-derived generated names remain available for compatibility. - Support multiple independent named systems/assets under one namespace-owning manager. Reuse injectable providers, file binding, bounded memory stores, optional transactional `IFileSystemProvider` persistence, state machines, telemetry and conformance infrastructure. - Keep one `docs/Vision.md` guide, titled `Machine Vision (OPC 40100-1 and OPC 40100-2)`, and one `Opc.Ua.Mcp.Vision` package. Include deterministic server/client samples. - Expose 17 Machine Vision MCP tools, including capabilities/status, bounded relationships and observation, complete small-file uploads/downloads, management, explicit control, conditions, and chunked transfers. Catalogue counts: 39 Vision-specific, 43 with connection tools, 81 Vision+Robotics, 155 Full. Every Machine Vision wire parameter has a schema description. **Compatibility and limits:** `AddVision()` remains Perception-only unless released parts are selected. Standalone Part 2 uses `AddVision().AddMachineVision()`; Machinery composition uses `AddVisionToMachinery()`. Existing tool defaults and command signatures are preserved. Observation owns and cleans up its subscription: 5 seconds/50 notifications by default, capped at 60 seconds/100 notifications. Complete-file tools are capped at 1 MiB; larger files use chunked transfers. Domain Error values remain distinct from UA service failures. **Versioning:** the seven Vision/MachineVision/MCP packages and their Debug variants follow the repository root version instead of being forced to preview. Vision.OpenUsd and its descendants remain preview-only. Development builds still inherit the root prerelease version. No release version, tag, release branch, publication policy, or feed promotion is changed. **Review actions:** all fourteen review threads have action/evidence replies and are resolved. Changes include the unified guide, Perception namespace, separate Part 2 packages and final naming, multi-system ownership explanation, interrupted-write cleanup, observer failure isolation, complete MCP parameter descriptions, and explicit package inventory. The preview-layout migration topic and the requested ReleaseProcess caveat were removed. The test Publish collector now retries recognized transient failures within one cancellation budget and propagates other exceptions. Commit |
||
|
romanettandClaude Opus 5.5
|
0716603543 |
Fix concurrent session creation on a shared ClientChannelManager channel (#4739)
# Description Sessions created concurrently through one `ClientChannelManager` against the same endpoint (`Session.CreateAsync(IClientChannelManager, ...)`) failed with `BadSecureChannelClosed 'The managed transport is closed.'`, and the log showed `Background operation OpenChannel started by ClientChannelManager failed. Channel is Closed.` Creating the same sessions one after another worked. ## Root cause A caller that finds an existing shared entry gets a lease right away, even while the entry's creator is still opening the transport: 1. That session's `OpenAsync` called `lease.Entry.SnapshotClientCertificate()` before any transport was installed and threw *"The managed transport is closed."* 2. The failed session disposed its lease. The refcount dropped to 0 and the entry was torn down. 3. The creator's in-flight `OpenInitialAsync` then found the entry closing and threw *"Channel is Closed."* Every session attached to that entry failed. ## Changes - **`Session.OpenAsync`** waits until the shared entry has an installed transport (new `ChannelEntry.WaitForTransportAsync`) before it reads the certificate. The signal fires when the initial open installs a transport, or when a reconnect that started during the initial open does. It does not wait for `Ready`: `OpenCoreAsync` also runs for session recreate inside a reconnect cycle, and the cycle installs the transport before participants reactivate. - **`ClientChannelManager.GetCoreAsync`**: the creator holds no lease while it opens the entry, so other participants' releases can close the entry under the open. The creator now retries on a fresh entry instead of failing. When the initial open fails on its own, the entry records it (`InitialOpenFaulted`, set under the entry lock at failure time, because the later lifecycle state cannot tell the two cases apart). A faulted open, or one closed by manager disposal, still fails the creator. ## Tests - `SharedChannelConcurrentSessionTests.SessionAttachedWhileChannelOpensWaitsForTransportAsync` (Client.Tests): holds the creator's transport through gated channel bindings until a second session has attached to the opening entry, then checks that both sessions open and share the channel. Fails deterministically without the fix. - `ClientChannelManagerManagedTests.WaitForTransportWaitsForReconnectStartedDuringInitialOpenAsync` and `CreatorDoesNotRetryInitialOpenThatFaultedAsync` (Core.Tests): cover the reconnect-during-open wait and the no-retry-on-fault path. - `SharedChannelConcurrentSessionTests.ConcurrentlyCreatedSessionsShareOneChannelAsync` (Client.Tests): creates 20 sessions concurrently through one manager against the reference server. It checks that they share one channel with refcount 20 and that every session can read. Fails without the fix. - `ClientChannelManagerManagedTests.CreatorRetriesWhenLeaseReleasedDuringInitialOpenAsync` (Core.Tests): holds the initial open with a mock transport, then attaches and releases a second lease during it. It checks that the creator gets a lease on a fresh entry. Fails without the fix with "Channel is Closed." Verification: - **Local runs (net10.0, sequential):** Opc.Ua.Client.Tests 2778 passed; Opc.Ua.Sessions.Tests 851 passed, 379 skipped; Opc.Ua.Core.Tests 6241 passed, 88 skipped. - **net48:** Client.Tests and Core.Tests compile cleanly. - **ServerLoadHarness repro:** `--client shared --sessions 20 --connect-parallel 4/20` now connects every run. The same build without the fix still fails. Note: when the creator's open is cut short this way, the `OpenChannel` background failure is still logged at `fail` level even though the creator now recovers. ## Related Issues - None filed yet. Found while load-testing the shared-channel client with the ServerLoadHarness. ## 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. - [ ] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
f5062be349 |
Complete each pooled service response at most once (#4729) (#4736)
## Proposed changes Fixes the cross-talk part of #4729 on master. `ServiceResponsePooledValueTaskSource` comes from one pool shared by the whole process, and `ManualResetValueTaskSourceCore.SetResult` never checks the version token. A second completion of an `EndpointIncomingRequest` therefore either threw `InvalidOperationException` or, once the source had been read and recycled, answered whichever request rented it next. That request could belong to another server instance in the same process. master's request queue (Channel + `FairRequestQueue`) already completes each request exactly once. But `ServerBase.ScheduleIncomingRequest` and `ProcessRequestAsync` are virtual, so a single override that completes and then throws (the `ProcessAsync` catch completes again) was enough to corrupt unrelated requests. - Each rental of the pooled source now gets a lease number, which is never reused for a later rental of the same instance. - `EndpointIncomingRequest` captures the lease when it is constructed and completes through `TrySetResult(lease, response)`. That is a single CAS on the pending lease, so only the first completion of the current rental is accepted. Late or stale completions are discarded and can never reach the next owner. - `SetResult`/`SetException`, which had no callers outside this path, are replaced by `TrySetResult`. The master378 backport #4738 also fixes the double completions in the old `RequestQueue` itself (snapshot in `Dispose`, and enqueue followed by a throw). ## Tests - `ServiceResponsePooledValueTaskSourceTests`: a second completion of the same rental is ignored, and a late completion after the source is recycled does not complete the next rental. Both use a private pool, so the result is deterministic. - `EndpointIncomingRequestTests.ProcessAsync_ScheduleCompletesThenThrows_KeepsFirstCompletionAsync`: a scheduler that completes the request and then throws. Without the fix it fails with `InvalidOperationException`. - `Opc.Ua.Core.Tests` Stack.Server + Stack.Tcp (net10.0): 1871 passed, 0 failed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
2c431ad728 |
Client/transport: one recv per small chunk, ManagedSession lock fast path, write coalescing (#4737)
Follow-ups from the "Not changed" list of #4720 (client per-request allocation and CPU): three changes that the paired A/B shows a gain for, plus a write-up of the fourth (discarded `ResponseHeader`), which is not changed. ## Results Paired A/B, each change against the commit before it: 4 alternating rounds, medians, 20 sessions, 24-core machine shared with other sessions. Client = the harness process, server = the reference server child. Allocated bytes per operation are deterministic; throughput and latency carry the run-to-run noise noted below. | Change | Scenario | Throughput | p50 | p90 | Client CPU / op | Client alloc / op | Server CPU / op | |---|---|---|---|---|---|---|---| | Receive staging | Read, 1 value | +4% | **−9%** | **−13%** | ≈ | ±0 | ≈ | | | Read, 1 value, SignAndEncrypt | +6% | −6% | −11% | ≈ | ±0 | ≈ | | | Read, 100 values | +2% | −4% | ≈ | ≈ | ±0 | ≈ | | | Data changes | ≈ | ≈ | ≈ | ≈ | ±0 | ≈ | | ManagedSession lock | Read, 1 value, ManagedSession | ≈ | ≈ | ≈ | ≈ | **−125 B** (2632 → 2507) | ≈ | | | Same, inflight 8 | ≈ | ≈ | ≈ | ≈ | −133 B | ≈ | | Write coalescing | Read, 1 value, 20 sessions on one shared channel, inflight 4 | **+13%** | **−13%** | −3% | −6% | ±0 | −4% | | | Same, inflight 1 | +20% ¹ | −8% | ¹ | −9% | ±0 | −13% | | | Read, 1 value, inflight 8 (one channel per session) | +2% | −4% | −6% | ≈ | ±0 | ≈ | | | Read, 100 values, inflight 8 | +3% | −4% | −8% | ≈ | ±0 | ≈ | | | Data changes | ≈ | ≈ | ≈ | ≈ | ±0 | ≈ | ≈: within run-to-run noise (about ±5% for CPU per op here). ¹ Rounds ranged from −42% to +32%; the inflight-4 runs were better in all four rounds. ## Changes - **`TcpByteTransport` receive**: `ReceiveChunkAsync` read the 8-byte chunk header and then the body, two `recv` calls per chunk. It now waits for data with a small staging buffer (8 KB, allocated on the first receive, not from the `BufferManager` budget). One receive returns the header and, when the chunk is small and already there, the whole chunk; bytes of following chunks are kept for the next call. A larger chunk is completed with a receive into the rented buffer, as before. - The receive buffer is still rented only after the header has been validated (message type, size against the negotiated limit), so an idle peer holds none of the budget. - A receive cancelled while its bytes are in the staging buffer keeps them for the next call; on .NET Framework the staging receive takes no token, so cancellation is observed once data arrives. - A receive that fails while reading the body of a chunk larger than the staging buffer has consumed part of the chunk; later receives then fail with `BadTcpInternalError` rather than read body bytes as a header. Before this PR such a failure lost the header and body bytes silently. The channel cancels receives only when it closes. - Server listener channels use the same transport. - **`ManagedSession` service calls**: each call was an async wrapper (await the reader lock, await the inner call), one more state machine box per request. - When services are available and no writer holds or waits for the lock, the reader lock is taken synchronously (`AsyncReaderWriterLock.TryEnterReader`). It is released by a cached continuation on the inner call's task, registered before the caller's await, so the lock is free again when the caller resumes. - Otherwise the call waits for the lock as before, so reconnect and failover still block new calls and drain the ones in flight. - The remaining 88 B per call is the task's continuation list (two continuations). - **Write coalescing** (`UaSCUaBinaryChannel`): when more writes for the same transport are queued behind the one being sent, the writer takes up to 16 of them and sends their segments in one vectored send. - Each write is then reported on its own, in order, with its size. A failed send is reported for every write in it, and their buffers are returned. - Writes still never start on the caller's stack, and one writer sends one batch at a time, so chunk order is unchanged. - Only byte-stream transports opt in (`TcpByteTransport`, also through the server's admission lease, via the internal `IUaSCChunkCoalescing`). WebSocket and capture decorators keep one send per chunk. ### Behaviour notes - Each TCP connection that has received holds an 8 KB staging array (pinned on .NET 5+), outside the `BufferManager` budget. The previous transport held an 8-byte header array. - `ManagedSession` service calls: if the inner call completes while the release continuation is being registered, the reader lock is released on the thread pool shortly after, instead of before the caller resumes. - Concurrent `ReceiveChunkAsync` calls on one `TcpByteTransport` now throw `InvalidOperationException`. The channel has a single receive loop, and interleaved receives on a stream were never valid. ## Not changed: discarded `ResponseHeader` Every generated structure's `Initialize()` constructs its structure-typed fields: `new ResponseHeader()` (112 B), `new RequestHeader()` (104 B). The decoders always create the instance (`new T()` or the activator) and then assign every field in `Decode`, so the default is thrown away. That is 4.6% of the client's 2.4 KB per Read and about 3% on the server, which discards a `RequestHeader` per decoded request and a `ResponseHeader` per response it constructs. `PublishResponse` also discards a default `NotificationMessage`. Options considered: - **Lazy getter** (`get => m_x ??= new X()`): `IsEqual`/`GetHashCode` compare fields, so a never-read default (null) would no longer equal an explicit default. Two threads reading an unset header could get different instances. Pooled instances reset through `Initialize()` anyway. - **Decode-only constructor** called by the activator: only the plain `Class`/`DerivedClass` templates assign every field in `Decode`, not optional-field or union types. A derived type's constructor must chain to a base built by an older generator in another assembly, which lacks it (the same problem `ForeignDerivedPooledExtensionClass` works around). It also needs a new public marker type in `Opc.Ua.Types`. - **Decode into the existing instance**: needs a new `IDecoder` member in all decoders. It also changes behaviour when a caller decodes into an object whose header it still references. Each touches every generated type for about 100 B per message, so this is left as is. ## Harness - `--client shared`: classic sessions whose secure channels come from one `ClientChannelManager`. - Created sequentially (`--connect-parallel`, default 1). Concurrent `Session.CreateAsync` calls on one manager fail with `BadSecureChannelClosed`, a separate bug. - 20 sessions on one channel reach about 45–70k req/s, against about 140k on 20 channels. - `ab.sh`: `CANDIDATE=` selects the candidate build. ## Testing - New: `TcpByteTransportTests`: chunks sent together, a chunk split across sends (inside the next header), a chunk larger than the staging buffer, cancellation keeping staged bytes. `UaSCBinaryChannelWriteQueueTests`: coalesced in order (1 + 16 + 16 + 7 sends), framing transport not coalesced, a failed batch reported for every write and later writes still sent. `AsyncReaderWriterLockTests`: `TryEnterReader` against a writer, the reader held until the operation completes and released on failure and on a completed operation, the awaiter resuming after the release. - Before the rebase onto master, net10.0, run sequentially: Core Stack+Security 4689 passed, 1 failed (`ClientBaseTests.ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`, a meter-listener flake fixed on master by #4708; passes 3/3 in isolation); Client 2768; Sessions 851 (379 skipped); Server 8398; Subscriptions 643. - After the rebase: Core Stack+Security 4811 passed; Client 2781; Stress Contract+Integration 45 passed; all 0 failed. Sessions, Server and Subscriptions were not rerun on the new base. - net48 build of `UA.slnx`: clean (0 warnings, 0 errors); `dotnet format whitespace` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
e54269e5bf |
Advance master to 2.0.2-preview (#4741)
# Description #4740 makes `release/2.0` the stable `2.0.1` release. This PR moves `master` from `2.0.1-preview.{height}` to `2.0.2-preview.{height}`, so its builds sort above the `2.0.1` release and stay on the preview channel. - **`version.json`**: `"version"` `2.0.1-preview.{height}` → `2.0.2-preview.{height}`. `versionHeightOffset` stays `0`. - **`preview-version.props`**: unchanged. `PreviewPackageBuildNumber` only applies when the root version is stable, so it has no effect on `master`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Copilotandromanett
|
373aced378 |
Add README status badges for scheduled CI workflows (#4734)
# Description This adds `README.md` status badges for the five scheduled workflows. Until now, the README had no badges for them. | Badge | Workflow | Schedule | |---|---|---| | Nightly | `nightly.yml` | Weekly, Sun 02:00 UTC | | Stress Tests | `stress-test.yml` | Daily 03:00 UTC | | Connection Stability | `stability-test.yml` | Daily 02:00 UTC | | Foreign Interop | `interop-foreign.yml` | Daily 03:17 UTC | | HA Long-Haul | `sample-ha-longhaul.yml` | Weekly, Sun 04:00 UTC | - Each badge uses `?branch=master&event=schedule`, so it shows only the result of scheduled runs on `master`. Manual (`workflow_dispatch`) runs and PR runs don't change the badge. - Each badge links to that workflow's run list, filtered by `branch:master event:schedule`. - The badges go after the existing CI / CodeQL / Coverage badges. - CodeQL also runs on a schedule, but its existing badge stays as it was and still covers all runs on `master`. ```markdown [](https://github.com/OPCFoundation/UA-.NETStandard/actions/workflows/nightly.yml?query=branch%3Amaster+event%3Aschedule) ``` ## Related Issues ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] 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. - [ ] 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. - [ ] 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. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: romanett <7413710+romanett@users.noreply.github.com> |
||
|
|
5e71e2243c |
Test that a secured connection refuses its JSON metadata announcement (#4724)
# Description #4712 makes a connection configured for message security send its DataSetMetaData announcement through the connection's security step, so the announcement is never sent unsecured. Its tests cover the UADP announcement. This adds the JSON case. The UADP security wrapper cannot protect a JSON message, so a connection whose WriterGroup uses `SignAndEncrypt` must refuse the JSON metadata announcement rather than publish the DataSet layout in plaintext. `MetaDataPublisherTests.JsonAnnouncementOfASecuredWriterGroupIsRefusedAsync` checks that nothing is published on the metadata topic and that the refusal is counted in `EncryptionErrors`. `PubSubApplication.StartAsync` returns after the initial announcement was sent or refused, so the check needs no wait. The test covers the fail-closed branch of `PubSubConnection.EncodeOutboundAsync` and the refused-send return of `SendNetworkMessageAsync`, two of the three changed lines the patch-coverage report of #4712 listed as uncovered. Test-only change. Review: the test double `RecordingTransport` guards its recorded sends with a private `System.Threading.Lock` and exposes `SendCount` and `GetSendsSnapshot()`, so the tests no longer lock the list itself ( |
||
|
|
b25429bccc |
Client: cut per-request allocation and CPU in the channel and session; measure the client in the load harness (#4720)
Follows #4707 (replaces #4714, which closed when the #4707 branch was deleted). Profiling the client process under load showed per-request allocation spread over the channel write path, the response wait, per-chunk header encoders and an AES instance per message, plus a stack walk per service call. This PR removes them and extends the load harness to measure the client. ## Results Paired A/B against the #4707 head, the server work now on master (3 alternating rounds, medians, 20 sessions unless noted, workstation GC on both sides, 24-core machine). Client = the harness process, server = the reference server child. | Scenario | Throughput | p50 | p90 | Client CPU / op | Client alloc / op | Server alloc / op | |---|---|---|---|---|---|---| | Read, 1 value | **+21%** (141k → 170k req/s) | −9% | −11% | −13% | −46% (4.5 → 2.4 KB) | −20% | | Read, 1 value, ManagedSession | **+18%** | −9% | −12% | −18% | −44% | −20% | | Read, 1 value, SignAndEncrypt | **+28%** | −14% | −20% | −24% | −52% | −26% | | Read, 1 value, 200 sessions | **+29%** | −11% | −17% | −17% | −46% | −20% | | Read, 100 values | +5% | −2% | −3% | −6% | −44% (15.6 → 8.8 KB) | −2% | | Read, 100 values, SignAndEncrypt | +8% | −7% | −2% | −2% | −46% | −3% | | Write, 100 values | ±0 | ≈ | ≈ | −14% | **−63%** (10.8 → 4.0 KB) | −2% | | Browse (Server object) | **+12%** | −7% | −11% | −19% | −17% | −12% | | Data changes, classic fast callback | ±0 ¹ | ≈ | ≈ | −21% | −10% | −5% | | Data changes, classic item events | ±0 ¹ | ≈ | ≈ | ≈ ² | −7% | −5% | | Data changes, ManagedSession (V2) | ±0 ¹ | ≈ | ≈ | ±0 | −5% | −5% | | Data changes, SignAndEncrypt | ±0 ¹ | ≈ | ≈ | −22% | −14% | −7% | ¹ Throughput is bounded by the writer rate; latency is dominated by the 100 ms publishing interval. Per notification the client uses about 2 µs and 170–260 B. ² The client uses 0.2–0.4 cores in the subscription scenarios; the CPU per notification is within run-to-run noise there. Client GC pause time in the window drops 38–66% in the Read and Write scenarios (17% for Browse). About 70% of the remaining client CPU per small request is kernel time in loopback socket send/receive. ## Changes - **Activity source**: `Session`, `SessionClientBatched`, the classic `Subscription` and `ClassicSubscriptionEngine`, and the TCP/HTTPS client channels resolve their `ActivitySource` once. `TelemetryExtensions.StartActivity` called `Assembly.GetCallingAssembly()` (a stack walk) on every service call, ~8% of client CPU. - **Channel write queue** (`UaSCUaBinaryChannel`, client and server): an ordered queue drained by one thread-pool writer replaces the per-write `ContinueWith`/`Unwrap`/trailing continuation chain. The writer is queued to the issuing thread's local pool queue, like the task continuation was (queuing it globally cost ~15 µs of p50 in a first iteration). Successful writes are reported inline instead of through `Task.Run`; failed writes still go through the pool. The client channel enters its gate only for a failed write instead of on every write completion. - **`TcpByteTransport`**: no linked `CancellationTokenSource` when the caller's token cannot be cancelled (the channel always passes `None`); a single-chunk `BufferCollection` uses the `ReadOnlyMemory` send instead of the vectored `Task<int>` send; `PoolingAsyncValueTaskMethodBuilder` on .NET 6+ for the receive/send state machines; no closure per vectored send. - **`BinaryEncoder.GetNodeIdEncoding`**: `Convert.ToUInt32(uint, IFormatProvider)` bound to the `object` overload and `Convert.ToByte(enum, ...)` boxed the enum, two boxes per encoded NodeId. 44% of the client allocation of a 100-value Read; the server encodes NodeIds the same way. - **Symmetric chunk headers**: the 24 header bytes are written and read with `BinaryPrimitives` instead of a `MemoryStream` + `BinaryEncoder`/`BinaryWriter` (send) and `BinaryDecoder`/`BinaryReader` (receive) per chunk. A chunk too short for a field still gets the decoder's `BadDecodingError`. - **AES-CBC**: `ChannelToken` keeps one `Aes` per direction, used with the one-shot `EncryptCbc`/`DecryptCbc` on .NET 6+ (net48 reuses the instance and creates the transform). Previously every message created an `Aes`, imported the key (`BCryptImportKey`) and created a transform, ~15% of client CPU with SignAndEncrypt. The instance is taken out of the token while in use, so concurrent callers never share one. The public `CryptoUtils` overloads are unchanged. - **`ChannelAsyncOperation`**: `SendRequestAsync` waits on an internal task-free completion source instead of `EndAsync` (`TaskCompletionSource` + `Task` + `WaitAsync` promise + state machine); `EndAsync` registers cancellation instead of `WaitAsync(ct)`; the timeout timer uses a static callback; completing with `default(T)` shares a box. - **Session keep alive**: every good response called `Timer.Change` under the session lock, taking the timer queue lock. It now records the time; the timer callback defers itself to one interval after the last response, which is when the per-response reset made it fire. ### Behaviour notes - `TcpByteTransport.SendChunkAsync`/`ReceiveChunkAsync` return pooled `ValueTask`s on .NET 6+: like any `ValueTask` they must be awaited once. - A successful write's `HandleWriteComplete` now runs on the writer instead of a separate pool work item. Overrides outside the stack that block in it delay later writes on that channel. - The write itself no longer flows the caller's `ExecutionContext` (`Activity.Current` is not set inside the transport send). ## Harness (`perf/`) - `client-process:` and `client-per-op:` result lines; `--trace-side client` records the harness itself. - `--client managed` (ManagedSession, V2 engine) and `--callback item` (classic per-item events). - Classic fixture sessions no longer request diagnostics in every response (the server otherwise sends a `DiagnosticInfo` per notification). - `ab.sh`: `SCENARIOS`/`ROUNDS`/`OUT`, CRLF tolerant, defaults before the scenario (a `--sessions 200` scenario silently ran with 20 before); `client-scenarios.txt`; `ab-summary.ps1` reports client and server medians. - README notes two profile traps: `Monitor.Enter_Slowpath` in the sampled CPU profile without matching contention events (46 in 10 s here), and allocation ticks over-attributing small objects allocated after large arrays. ## Testing - New: `UaSCBinaryChannelWriteQueueTests` (order, one send at a time, never on the caller's stack, failures reported and later writes still sent, concurrent writers), `ChannelAsyncOperationWaitTests` (late/early completion, faults, cancellation, disposal, continuation off the completing thread, 2000-iteration completion/cancellation race, timeout), `CryptoUtilsAesCacheTests` (cached = fresh cipher text across IVs, round trips, concurrent users, token disposal), `TcpByteTransportTests` single/multi-buffer and cancellable sends, `SessionKeepAliveTests` keep-alive read exactly one interval after the last response. - After merging master: Core Stack 3571 passed, Client 2763 passed, 0 failed. - Before merging master, net10.0: Core Stack+Security 4680 passed; Client 2763; Sessions 849 (379 skipped); Server 8391; Subscriptions 643; all 0 failed. - Stress Contract+Integration: 5 failures, the same 5 on the unchanged #4707 head (`CertRotationContractTests`, `GateAndBypassTests`, `LeaseLifecycleTests`, `RetryBudgetEnforcementTests` reflection seam, `CertRotationLiveTests`), not caused by this PR. - net48 build of `UA.slnx` clean; `dotnet format whitespace` clean. ## Not changed (follow-ups) - Two `recv` calls per chunk (8-byte header, then body). Merging them needs read-ahead with carry-over, which the transport avoids so that idle peers do not hold receive buffers. - `ManagedSession` service wrappers add an async state machine and a reader lock per call (~200 B). - Generated responses allocate a discarded `ResponseHeader` (generator-wide, as noted in #4707). - Decoded `MonitoredItemNotification` objects and the V2 `DataValueChange[]` per message dominate notification allocation (`WithPoolNotifications` pools the latter). - Coalescing queued writes on one channel into one vectored send would cut syscalls under concurrent requests on a shared channel. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc <marcschier@hotmail.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> |
||
|
|
c7fdeb6de1 |
Fix stress Contract/Integration failures and run them in stress-test.yml (#4718)
## Description `Opc.Ua.Stress.Tests` with `Category=Contract|Category=Integration` had five failures on master. The README says these categories run on every PR, but no workflow ran them: `get-ci-matrix.ps1` excludes the Stress project, and `stress-test.yml` only ran `ChaosTCP`. The old `Channels.Stress` special case in `buildandtest.yml` never matched the project name, and #4513 removed it. So the tests rotted unseen. Two of the five failures were real product bugs. ### Product fixes - **Session recreate after a server restart failed forever (regression from #4503).** User-token PolicyIds are assigned by the server and are not stable across a restart. In the failing run the client had cached `Anonymous|20` and the restarted server offered `Anonymous|2`. The CreateSession check added in #4503 requires an exact match, including PolicyId, so every `RecreateInPlace` failed with `BadSecurityChecksFailed` ("different security policy for the selected user identity token"). The managed session then retried without end. - `Session.OpenCoreAsync` now adopts the server's policy when only the PolicyId differs. TokenType, SecurityPolicyUri, IssuedTokenType and IssuerEndpointUrl must still match, so a downgrade is still rejected. The token is restamped with the new id. - **A certificate rotation during a reconnect reactivated every session twice.** `ChannelEntry.ReconnectCycleAsync` only noticed the superseded client certificate *after* it had notified participants. It then replaced the transport and notified them again. The check now also runs right after the transport reconnects, before participants are notified. ### Tests updated to the #4516 contracts - `RetryBudgetEnforcementTests`: `Session.ReconnectManagedChannelAsync` gained an optional reverse-connection parameter, which the reflection call must supply. - `GateAndBypassTests`: the participant callback token is cancelled once `OnReconnectAsync` returns. The spawned worker now runs on the test token. - `LeaseLifecycleTests`: releasing the last lease cancels the recovery in progress, as documented. The reconnect caller now expects `OperationCanceledException`. ### CI - New `Contract and Integration` job in `stress-test.yml`, with the same gate as Chaos TCP: nightly, manual, and PRs labelled `stress`. It takes about 1 minute of test time. - The README now says where these categories actually run. ### Tests - New `SessionIdentityReviewTests.CreateAdoptsRenumberedUserTokenPolicyIdAsync`, covering both a renumbered id (accepted) and a changed token security policy (rejected). It fails without the fix. - Each of the 5 fixed tests passed 3/3 runs in isolation. The full `Contract|Integration` filter passed 45/45 twice in Debug and once in Release. - Related tests also pass: Core.Tests channel tests 587/587, Sessions.Tests channel tests 119 passed (63 skipped), Client.Tests channel/session/reconnect tests 512/512. - The net48 build of `UA.slnx` is clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> |
||
|
Cristi PogaceanandCopilot
|
9344354aaa |
Fix empty LocalizedText binary field encoding (#4709)
# Description Omit empty `Locale` and `Text` fields from the OPC UA Binary DataEncoding for `LocalizedText`, as required by OPC 10000-6, Section 5.2.2.14, Table 24. Add wire-format tests for empty and non-empty fields, including translation-backed localized text. Validation: - `git diff --check` passed before commit. - `dotnet build tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj --framework net10.0 --no-restore --verbosity minimal -p:NuGetAudit=false` could not complete because access to `https://api.nuget.org/v3/index.json` failed with `NU1301` and a TLS handshake error. ## Related Issues No related issue. ## 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. - [ ] 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. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
9099aee13e |
Fail CI when a test fixture's setup or teardown fails (#4723)
# Description
`get-test-verdict.ps1` accepts a non-zero `dotnet test` exit whenever
the TRX counters show no failure, on the assumption that the host died
at exit after every teardown had run. A failed fixture setup or teardown
breaks that assumption. The assembly-level `[OneTimeTearDown]`
certificate leak check fails after its tests passed, no TRX counter
records it, and the host exits 1. In the last 10 master runs, CI
accepted exactly that as an "at-exit stall" for `Opc.Ua.Gds.Tests` and
`Opc.Ua.Server.Tests` on every leg (net48, and net10.0 on Windows,
Linux, and macOS). #4719 fixed the Server.Tests leak; #4721 fixes the
GDS leak.
The NUnit adapter reports such a failure as a TRX run message, `<RunInfo
outcome="Error"><Text>TearDown failed for test fixture
Opc.Ua.Gds.Tests.LeakDetectionSetup</Text></RunInfo>`, followed by a
second one with the failure message. For a setup failure it writes
`Setup failed for test fixture …`.
Changes:
- `Measure-TestResults` collects these messages. `Get-TestRunVerdict
-FixtureFailures` rejects a run that has any of them, whatever the exit
code, and names the fixture in the reason.
- A host that dies at exit after a clean run is still tolerated. The
only other tolerated case in the last 10 master runs,
`Opc.Ua.Client.Tests` on net48 in run 37835429177, logged "Test host
process crashed" and stays tolerated.
- `Measure-TestResults` and `Get-CounterValue` move from
`run-dotnet-tests.ps1` to the dot-sourced `get-test-verdict.ps1`, so
`CiTestVerdictTests` can feed them TRX files. The source check of the
counter set becomes a behavioral test.
- `docs/DeveloperGuide.md` describes the rule.
**Merge order:** after #4727 (#4719 and #4721 are on master). With #4721
merged, this PR's CI still rejects `Opc.Ua.Gds.Tests` on net48: eight
`PushTest` fixtures fail in `TearDown` and leak 88 certificates when
their setup is ignored, which master accepted as an "at-exit stall".
#4727 fixes them. On every other leg, `Opc.Ua.Gds.Tests` now passes
under the rule.
### Validation
| Command | Result |
| --- | --- |
| `dotnet test tests/Opc.Ua.Tools.Tests -f net10.0 --filter
FullyQualifiedName~CiTestVerdictTests` with the scripts from master | 6
new tests fail, 13 pass |
| Same with this change | 19 passed |
| `Measure-TestResults` and `Get-TestRunVerdict` on the TRX of a full
`Opc.Ua.Gds.Tests` run without #4721, exit code 1 | Rejected: `1 test
fixture setup or teardown failure(s): TearDown failed for test fixture
Opc.Ua.Gds.Tests.LeakDetectionSetup.` |
| Same on the TRX of a run with #4721, exit code 1 | Tolerated, as
before |
| CI of this PR before #4721 merged | `Opc.Ua.Gds.Tests` rejected on all
four legs for `TearDown failed for test fixture
Opc.Ua.Gds.Tests.LeakDetectionSetup`; every other project passed |
| CI after merging master with #4721 (
|
||
|
Cristi PogaceanandCopilot
|
2cdf0de97f |
Fix scoped certificate-store resolution in GDS push operations (#4726)
# Description Fix the remaining GDS push certificate paths that bypass instance-scoped certificate stores, while preserving OPC UA 1.05.07 trust and transaction requirements. - Use the configured resolver/registry for previous-key loading, slot resolution, application-store commit/rollback, issuer import/compensation, and rejected-store opening. - Validate application-certificate uploads through the configured manager and the group's registered trust scope, instead of a temporary manager that cannot see custom providers. - Isolate per-call `AcceptError` validation cores from normal cached trust decisions. Part 12 installation-time acceptance must neither reuse nor populate the connection-validation cache. - Compensate the application-certificate swap if opening the issuer store fails, not only if importing an issuer fails. - Add scoped-provider, cross-group trust, cache-isolation, and compensation regressions. Reuse the portable PEM writer and short temporary PKI paths so the tests work on .NET Framework too. - Use source-generated failure logging without private-key bytes, and document custom-store/pending-key requirements. The branch starts at master `7f14fb864fc93532f3dbab1e1b5e2ba9b5f605c5`. It excludes the unrelated LocalizedText commits, the local handoff, PLC changes, logs, keys, certificates, and build outputs. No public API, dependency, package version, global provider registration, or analyzer configuration is changed. ### Specification review The online Part 12 reference identifies version **1.05.07**. This is a focused review and regression validation of the changed behavior, **not a claim of complete stack conformance or CTT certification**. | Requirement | Review / evidence | | --- | --- | | [Part 12, 7.10.5](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.5): validate application certificates using the associated TrustList; ignore the supplied issuer list | Existing issuer-argument regressions retained. New flat-layout scoped-provider cases accept a CA installed in the selected trusted/issuer store and reject one installed only in another scope. | | [Part 4, 6.1.3](https://reference.opcfoundation.org/specs/OPC-10000-4/6.1.3) and Part 12, 7.10.5: suppressible installation errors must not bypass normal peer trust or non-suppressible failures | Missing-issuer, signature/revocation, explicit-trust, and validation-core tests; new two-way cache-isolation cases with `UseValidatedCertificates = true`; normal peer validation remains untrusted after issuer-only push acceptance. | | Part 12, 7.10.5: matching private key, encrypted channel and SecurityAdmin | Existing access/key/type tests retained; scoped own-store upload cases cover no uploaded key and PEM. PFX and regenerated-key cases remain in the selected suite. | | [Part 12, 7.10.2](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.2), [7.10.9](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.9), and [7.10.11](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.11): staging, ApplyChanges, cancellation, ownership and compensation | Push/transaction/rollback suites; new custom-provider compensation cases cover both issuer-import and store-open failure, retaining the old application certificate and pre-existing issuers. | | [Part 12, 7.10.7](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.7): do not delete endpoint-referenced certificates | Existing ApplyChanges endpoint-reference and delete/create replacement tests retained. | | [Part 12, 7.10.10](https://reference.opcfoundation.org/specs/OPC-10000-12/7.10.10): CSR key handling and persistence | Existing-key scoped CSR test, SDK pending-key/cancel cases, and supporting PLC encrypted round trips/restarts. Arbitrary custom backends still require a durable pending-key implementation; the separate PLC implementation is not included here. | ### Validation Windows, .NET SDK 10.0.401, Release, NUnit/VSTest. Build analyzers remained enforced: **no `WarningsNotAsErrors`, suppressions, or analyzer downgrades**. `NuGetAudit=false` was needed because the local nuget.org TLS connection failed; no repository audit setting was changed. | Selection | Result | | --- | --- | | SDK GDS push/trust/transaction/rollback, net10.0 | **192 passed, 0 failed** | | SDK certificate-manager/provider/explicit-trust/validation-core, net10.0 | **180 passed, 0 failed** | | SDK certificate-manager/provider/explicit-trust/validation-core, net48 | **180 passed, 0 failed** | | SDK GDS push/trust/transaction/rollback, net48 | **192 passed, 0 failed** | | Final portable-PEM/compensation tests, net10.0 | **5 passed, 0 failed** | | Supporting PLC GDS/pending-key/compatibility selection, net10.0 | **29 passed, 0 failed** against this SDK source | The PLC tests exercise encrypted UA calls, signed PEM/CSR updates, persistence, fresh connections and restarts across Directory, FlatDirectory and KubernetesSecret providers. The Kubernetes API is an in-memory double, **not a real Kubernetes deployment**. PLC tests and its durable pending-key implementation are local companion-repository changes, not SDK PR contents or independently reproducible CI evidence from this branch alone. Commands below use the same isolated artifact locations as the validation run, expressed relative to the user profile: ```powershell $files = Join-Path $env:USERPROFILE '.copilot\session-state\003a7438-0312-4c10-beed-cd95c0fe400e\files' $sdk = Join-Path $files 'sdk-build' $net48 = Join-Path $files 'solution-net48' $push = 'FullyQualifiedName~ConfigurationNodeManagerPushTests|FullyQualifiedName~ConfigurationNodeManagerPushValidationTests|FullyQualifiedName~PushComplianceRegressionTests|FullyQualifiedName~PushRollbackCancellationRegressionTests|FullyQualifiedName~PushConfigurationTransactionCoordinatorTests' $manager = 'FullyQualifiedName~CertificateManagerTests|FullyQualifiedName~CertificateManagerExplicitTrustTests|FullyQualifiedName~CertificateManagerProviderResolutionRegressionTests|FullyQualifiedName~CertificateValidationCoreTests' # Passed: 192/192. dotnet test .\tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f net10.0 -m:1 --no-restore -p:GeneratePackageOnBuild=false -p:UseArtifactsOutput=true "-p:ArtifactsPath=$sdk" -p:NuGetAudit=false --filter $push # Passed: 180/180. dotnet test .\tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -c Release -f net10.0 --no-build --no-restore -p:UseArtifactsOutput=true "-p:ArtifactsPath=$sdk" --filter $manager # Passed: 180/180. dotnet test .\tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -c Release -f net48 --no-build --no-restore -p:CustomTestTarget=net48 -p:UseArtifactsOutput=true "-p:ArtifactsPath=$net48" --filter $manager # Passed: 192/192 after the portable PEM writer and shorter temporary PKI paths. dotnet test .\tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f net48 -m:1 --no-restore -p:CustomTestTarget=net48 -p:GeneratePackageOnBuild=false -p:UseArtifactsOutput=true "-p:ArtifactsPath=$net48" -p:NuGetAudit=false --filter $push # Passed: 5/5 after final test portability changes. dotnet test .\tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -c Release -f net10.0 -m:1 --no-restore -p:GeneratePackageOnBuild=false -p:UseArtifactsOutput=true "-p:ArtifactsPath=$sdk" -p:NuGetAudit=false --filter 'FullyQualifiedName~CertificatePushUsesScopedOwnStoreProviderAsync|FullyQualifiedName~ApplyCertificateSlotChangeSelfCompensatesWhenIssuerImportFailsAfterAppCertSwapAsync' # Passed: 29/29; the companion project and this SDK had been built together first. dotnet test C:\src\iot-edge-opc-plc\tests\opc-plc-tests.csproj -c Release --no-build --no-restore -p:UseLocalOpcUaStack=true -p:OpcUaSourceRoot=C:\src\UA-.NETStandard -p:UseArtifactsOutput=true "-p:ArtifactsPath=$sdk" --filter 'FullyQualifiedName~GdsPush|FullyQualifiedName~PlcPendingCertificateKeyStoreTests|FullyQualifiedName~PreviewCompatibilityTests' ``` The `--no-restore` / `--no-build` invocations require prior restoration/build. The initially empty artifact-directory `--no-restore` invocation produced no test execution and is **not** counted. Initial compile/test portability failures were fixed before the final results above. Full-solution attempts did **not** pass: ```powershell # Failed/incomplete: AMB server startup timed out in two tests; stopped after failures. dotnet test .\UA.slnx -c Release -f net48 -m:1 -p:CustomTestTarget=net48 -p:GeneratePackageOnBuild=false -p:UseArtifactsOutput=true "-p:ArtifactsPath=$net48" -p:NuGetAudit=false # Failed: NU1301 restoring OpenUsd-related projects due to nuget.org TLS handshake failure. dotnet test .\UA.slnx -c Release -f net10.0 -m:1 -p:CustomTestTarget=net10.0 -p:GeneratePackageOnBuild=false -p:UseArtifactsOutput=true "-p:ArtifactsPath=$(Join-Path $files 'solution-net10')" -p:NuGetAudit=false ``` A retry through an already configured local package source progressed further, but the OneFuzz nested publish did not inherit the audit override and caused **210 setup failures (NU1900)**; the broad run was stopped. These runs are not a clean full-suite result. AMB startup failures have not been baseline-qualified and are not claimed to be fixed by this PR. `git diff --check` passed. A changed-file whitespace check reported seven pre-existing diagnostics at lines 371-424 of `CertificateManagerTests.cs`, outside this change (new tests start at line 563); no unrelated formatting sweep was made. **Remaining gates:** successful full solution on net48/net10.0, quantitative coverage, Linux, required CI/CodeQL, and applicable CTT/interoperability qualification. Keep this PR draft until the required gates are resolved; the tests above do not establish 100% OPC UA compliance. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4725. - Follow-up to #4546, #4530 and #4531 for remaining certificate-push consumers. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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. Tests were added and pass as recorded, but coverage was not measured. CLA status is not asserted on behalf of the contributor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
Marc SchierandCopilot
|
09ca425b3a |
Make the encoder stack-depth tests hold on a reused thread stack (#4722)
# Description `EncoderStackDepthTests.DecodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Json, VariantArray)` and `(Json, DataValue)` fail intermittently on Linux CI, for example in run 37921301898, attempt 1, `test-linux-net10.0 (12/15)`. The 450-level JSON input decodes without tripping the stack check, so the expected `BadEncodingLimitsExceeded` is missing. A re-run passes. Cause: a thread that asks for a 256 KB stack can run on more. glibc reuses the cached stack of an exited thread when it is at most four times the requested size, and the fixture's own 1 MB threads leave such stacks behind. `RuntimeHelpers.EnsureSufficientExecutionStack` measures the stack the thread actually got, and 450 JSON levels fit a 1 MB stack. The product's stack check is correct; the test relied on the requested size. Deepest nesting that fits before the check fires (net10.0, Release): - On a 256 KB thread: about 100 JSON levels on Linux and 70 on Windows. - On a 1 MB thread, Linux: 548 levels for a JSON Variant array, and up to 2477 for a JSON EncodeableTree. Encoding a Binary EncodeableTree fits 1899 levels, so the 2000 levels of the other codecs had no margin either. Fix: - The unbounded-limit decode and encode tests run on a 1 MB and a 256 KB stack: the largest stack glibc can hand a 256 KB request, and the requested size. - Every codec nests 5000 levels, twice the deepest nesting that fits a 1 MB stack. - The JsonEncoder cannot nest deeper than 1000 JSON levels, so the JSON input is written by hand. `NestedJsonMatchesTheJsonEncoder` checks it against the encoder's output for every shape. ### Validation | Command | Result | | --- | --- | | Original test logic in the `mcr.microsoft.com/dotnet/sdk:10.0` container, each decode right after a 1 MB thread exits, 20 times per shape | Decode succeeded 20 of 20 times for VariantArray and DataValue, the CI failure; 0 of 20 without the 1 MB thread, and 0 of 20 on Windows either way | | `dotnet test tests/Opc.Ua.Types.Tests -f net10.0 --filter FullyQualifiedName~EncoderStackDepthTests`, Release and Debug on Windows, 2 runs each | 80 passed | | Same in the Linux container (Release), 3 runs | 80 passed | | Same with `-f net48` | 80 passed | | Linux, depth temporarily set back to 2000 | `EncodeWithUnboundedNestingLimitFailsBeforeTheStackOverflows(Binary,EncodeableTree)` fails on the 1 MB stack | | Linux, `EncodingLimits.EnsureSufficientStack` temporarily disabled | The test host crashes with a stack overflow, so the tests still catch a missing check | ## Related Issues - No tracking issue. Seen on the CI of #4710. ## Checklist - [ ] 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. - [ ] 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> |
||
|
Marc SchierandCopilot
|
0abe194b25 |
Dispose the certificate leaked by PushComplianceTests (#4721)
# Description `dotnet test tests/Opc.Ua.Gds.Tests` exits 1 even when every test passes. The assembly-level `LeakDetectionSetup` teardown reports `Certificate leak detected: 1 instance(s) created but not disposed`. CI accepts the exit code as an "at-exit stall" and only warns: the last 10 master runs show the warning for `Opc.Ua.Gds.Tests` on all four legs (net48, and net10.0 on Windows, Linux, and macOS). Cause: with `OPCUA_CERTIFICATE_LEAK_TRACKING=1`, the leak report's allocation stack names `PushComplianceTests.CreateSelfSigned()`, called inline in `TrustListTransactionLifecycleAsync`. The certificate passed to `AddCertificateAsync` was never disposed. Fix: the test creates the certificate with a `using` declaration, like every other certificate in the fixture. This is a test-only change. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.Gds.Tests -f net10.0 --logger trx` with `OPCUA_CERTIFICATE_LEAK_TRACKING=1`, before the fix | 1258 passed, 0 failed, but exit 1: `TearDown failed for test fixture Opc.Ua.Gds.Tests.LeakDetectionSetup`, `Certificate leak detected: 1 instance(s) created but not disposed (created=70028, disposed=70027)`, allocated in `PushComplianceTests.CreateSelfSigned()` | | `dotnet test tests/Opc.Ua.Gds.Tests -f net10.0 --filter "FullyQualifiedName~PushComplianceTests.TrustListTransactionLifecycleAsync"`, before the fix | 1 passed, exit 1 with the same leak report | | Same, after the fix | 1 passed, exit 0 | | `dotnet test tests/Opc.Ua.Gds.Tests -f net10.0` with leak tracking, after the fix | 1258 passed, 0 failed, 56 skipped; exit 0 | Not run locally on net48: on this machine the fixture's `OneTimeSetUp` fails on net48 with `BadSecurityChecksFailed` (the server cannot verify the push client's OpenSecureChannel request) before any test body runs, and the `PushTest` fixture logs the same error. CI's net48 leg runs the fixture. ## Related Issues - No tracking issue. Found while surveying the CI "at-exit stall" tolerance; #4719 fixes the same class of leak in `Opc.Ua.Server.Tests`. A follow-up makes CI fail on such teardown failures. ## Checklist - [ ] 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. - [ ] 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. - [ ] 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> |
||
|
Marc SchierandCopilot
|
7f14fb864f |
Secure the UADP DataSetMetaData announcement on secured connections (#4712)
# Description `MetaDataPublisher` encoded the DataSetMetaData announcement itself and called `IPubSubTransport.SendAsync` directly. That bypassed the connection's security wrapper and its fail-closed check. A WriterGroup configured for `SignAndEncrypt` therefore announced its DataSet layout (field names, types, version) unsigned and unencrypted. A secured subscriber dropped the announcement (`Dropping unsecured inbound frame ... requiring SignAndEncrypt`, documented as an expected warning in docs/PubSub.md), and any listener could read it. OPC 10000-14 §6.2.5.2 applies the WriterGroup's SecurityMode to the NetworkMessages it publishes, and §7.2.4.6.3 carries the security of a discovery announcement in its SecurityHeader. Confirmed on the wire: the documented `udp-uadp` exchange sent secured DataSetMessages plus one DiscoveryResponse (announcement type 2, DataSetMetaData) with ExtendedFlags1 `0x81`, meaning the SecurityHeader bit was clear. Review feedback widened the scope to every UADP discovery message of a secured connection, and to the diagnostics of secured messages. Fix: - `MetaDataPublisher` builds the announcement and hands it to the connection (`PubSubConnection.SendMetaDataAnnouncementAsync`). - One encode-and-secure step (`PubSubConnection.EncodeOutboundAsync`) now serves data messages, discovery requests and responses, and the announcements that UDP and Ethernet send to their transport-specific discovery destination (ApplicationInformation, PublisherEndpoints, PubSubConnection). On a connection configured for message security, it applies the SecurityMode (`Sign`, or `SignAndEncrypt`) to UADP messages and refuses to send any message it cannot secure. - Before, a secured connection dropped its own discovery requests, so a secured subscriber could not ask for metadata, writer configuration, or endpoints. It also refused metadata and writer-configuration probe responses, and sent the transport-specific announcements in plaintext. - A discovery response or announcement that cannot be secured or sent is logged (`DiscoveryMessageNotSent`, Warning) and dropped. Securing announcements makes them depend on a current key; without this, a key provider with no key yet, for example before a Security Key Service delivers the first key, made `StartAsync` fail, and a probe response that could not be secured ended the receive loop. Requests sent through `RequestDiscoveryAsync` still throw to the caller. - Secured messages are counted in `SentNetworkMessages` and `SentDataSetMessages` like unsecured ones. Before, they bypassed the encoder that counts them, so secured data messages were never counted. - `UadpDiscoveryCoder` can encode a discovery message with the SecurityHeader bit set, and reports the security boundary after the PublisherId (and DataSetClassId), which is where `UadpDecoder.TryReadOuterPrefix` ends the prefix. `UadpEncoder.EncodeWithSecurityBoundary` accepts discovery messages. - The receive path is unchanged: it already unwraps secured discovery frames and registers authenticated metadata. - Not changed: the MQTT last-will status message is registered with the broker when the connection opens and published later, so it cannot carry a current SecurityHeader; it stays unsecured. - docs/PubSub.md no longer lists the unsecured-frame warning. It describes how the SecurityMode applies to announcements and discovery (`Sign` authenticates; `SignAndEncrypt` also encrypts) and the last-will exception. Tests: - `MetaDataPublisherTests.UadpPathSecuresTheAnnouncementOfASecuredWriterGroupAsync`: the announcement of a SignAndEncrypt WriterGroup carries a SecurityHeader, does not contain the DataSet name in cleartext, and unwraps and decodes with the subscriber's keys. Before the fix, the announcement had no SecurityHeader. - `MetaDataPublisherTests.SecuredNetworkMessagesAreCountedAsSentAsync`: before the fix, `SentNetworkMessages` read 0 on a secured connection. - `PubSubDiscoveryTests.SecuredSubscriberDiscoversSecuredPublisherWithoutPlaintextFramesAsync`: a secured subscriber and a secured publisher, connected through an in-memory transport that also sends announcements to a separate destination, discover metadata, writer configuration, and endpoints, and no frame lacks a SecurityHeader. Before the fix, the metadata request was dropped. With only the announcement change reverted, the PublisherEndpoints response was dropped. - `PubSubDiscoveryTests.StartupAnnouncementsWithoutACurrentKeyDoNotFailTheStartAsync` and `ReceiveLoopSurvivesADiscoveryResponseThatCannotBeSecuredAsync`: both failed before the fail-soft change. - `UadpDiscoveryTests.SecuredDiscoveryMessagesSplitWhereTheReceiverExpects`: for requests and responses, with and without a DataSetClassId, the secured encoding differs from the unsecured one only by the SecurityHeader flag, and its boundary equals the receiver's prefix length. - `UadpDiscoveryTests.EncodeWithSecurityBoundaryRejectsAMessageItCannotSecure`. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 1498 passed, 1 skipped (the UDP multicast loopback discovery test, which this host skips) | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net48 -p:CustomTestTarget=net48` | 1498 passed, 1 skipped | | `dotnet test` on net10.0 for `Opc.Ua.PubSub.Udp.Tests`, `.Eth.Tests`, `.Mqtt.Tests`, `.Server.Tests`, `fuzzing/Opc.Ua.PubSub.Fuzz.Tests` | 234, 55, 157, 177, 149 passed | | `dotnet test` on net10.0 for `Opc.Ua.PubSub.Kafka.Tests`, `.Adapter.Tests`, `.Diagnostics.Tests`, `Opc.Ua.Redundancy.PubSub.Tests` (on |
||
|
Marc SchierandCopilot
|
60dc5ab404 |
Reset the push coordinator after each PushComplianceRegressionTests test (#4719)
# Description `dotnet test tests/Opc.Ua.Server.Tests` exits 1 even when every test passes. The assembly-level `LeakDetectionSetup` teardown reports `Certificate leak detected: 3 instance(s) created but not disposed`. Cause: `CloseAndUpdateRejectsRetainedCrlOfRemovedCaAsync` and `CloseAndUpdateAcceptsVersion3CertificateWithoutExtensionsAsync` stage TrustList updates through `CloseAndUpdate` and finish without `ApplyChanges` or `CancelChanges`. `TearDown` disposed the TrustLists but not the staged operations, so the `PushConfigurationTransactionCoordinator` kept the staged and original `CertificateCollection`s undisposed. CI did not catch it: `.github/scripts/get-test-verdict.ps1` accepts a nonzero exit when the TRX counters show no failures, and reports it only as an "at-exit stall" warning. Fix: `TearDown` calls `m_coordinator.Reset()`, which discards the staged operations and disposes their collections. This is a test-only change; in production, a session's staged operations are applied, cancelled, or discarded when the session closes. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~PushComplianceRegressionTests"`, before the fix | 15 passed, but exit 1: `TearDown failed for test fixture Opc.Ua.Server.Tests.LeakDetectionSetup`, `Certificate leak detected: 3 instance(s) created but not disposed (created=49, disposed=46)` | | Same, after the fix, 3 runs | 15 passed, exit 0, no leak report | | Same with `-f net48 -p:CustomTestTarget=net48` | 15 passed, exit 0 | | `dotnet test tests/Opc.Ua.Server.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 8386 passed, 0 failed, 5 skipped; exit 0 | ## Related Issues - No tracking issue. Found while explaining the exit code 1 of a full `Opc.Ua.Server.Tests` run for #4716. ## Checklist - [ ] 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. - [ ] 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. - [ ] 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> |
||
|
Marc SchierandCopilot
|
aeeef5b420 |
Send UDP multicast on the network interface the transport joins (#4711)
# Description `UdpDatagramTransport` joined multicast groups on the resolved network interface: the connection address's `NetworkInterface`, the `PreferredNetworkInterface` option, or the first operational interface. It never set `IP_MULTICAST_IF` / `IPV6_MULTICAST_IF`, though, so multicast data and discovery announcements left through the operating system's default multicast route. On a computer with several network interfaces, a publisher configured for one network sent on another, and subscribers on the configured network missed the messages (OPC 10000-14 §7.3.2.2, `NetworkAddressDataType.NetworkInterface`). `ConfigureSocket` now sets the multicast send interface to the resolved interface for IPv4 and IPv6 sockets. That includes the separate socket that sends discovery announcements for loopback-bound connections. A failure is logged at Debug level, like the other socket options. **Behavior change:** without a configured interface, multicast now leaves through the first operational non-loopback interface, which is the interface subscribers join on, instead of the default multicast route. `docs/PubSub.md` and the XML docs of `PreferredNetworkInterface` and `UdpNetworkInterfaceResolver` describe this and recommend naming the interface on multi-homed computers. Tests (`UdpDatagramTransportMulticastInterfaceTests`): - The IPv4 and IPv6 egress options equal the resolved interface. Before the fix they were `0.0.0.0` and `0`. - A publisher and subscriber pinned to the loopback interface exchange a payload. A raw control sender tells a host without loopback multicast apart from a publisher that sends elsewhere. The test fails with the bug on Linux; Windows also delivers local copies sent on other interfaces, so there it only confirms the exchange. ### Validation | Command | Result | | --- | --- | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0 -p:CustomTestTarget=net10.0 --filter "FullyQualifiedName~UdpDatagramTransportMulticastInterfaceTests"`, with the fix temporarily disabled | IPv4 and IPv6 option tests failed (`Expected: <192.168.178.101> But was: <0.0.0.0>`, `Expected: 4 But was: 0`) | | The same tests from the Windows-built binaries in a Linux container (`mcr.microsoft.com/dotnet/sdk:10.0`, `dotnet test Opc.Ua.PubSub.Udp.Tests.dll`), fix disabled, 5 runs | 3 of 3 failed in every run, including the loopback exchange (the control sender arrived, the publisher's multicast did not) | | Same, with the fix, 5 runs | 3 of 3 passed in every run | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 237 passed (Windows, and the same binaries in the Linux container) | | `dotnet test tests/Opc.Ua.PubSub.Udp.Tests -f net48 -p:CustomTestTarget=net48` | 232 passed | | `dotnet test tests/Opc.Ua.PubSub.Tests -f net10.0 -p:CustomTestTarget=net10.0` | 1490 passed, 1 skipped | | `dotnet build src/Opc.Ua.PubSub.Udp -c Release` (net10.0, net9.0, net8.0, net48) | 0 warnings | | The docs/PubSub.md first exchange (unicast loopback), run verbatim, with #4710 applied | 11 DataSets; the unsecured-frame warning appears once, as documented | ## Related Issues - No tracking issue. Found while verifying the PubSub first-exchange documentation (follow-up to #4555). ## Checklist - [ ] 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. - [ ] 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> |
||
|
|
9600f0de9c |
Let servers start in globalization-invariant mode (#4716)
# Description
A server did not start in globalization-invariant mode
(`InvariantGlobalization`, or
`DOTNET_SYSTEM_GLOBALIZATION_INVARIANT=1`), which Native AOT and
container images often enable. When the server loaded its default status
texts, `ResourceManager` created `CultureInfo("en-US")`. In this mode
the runtime throws `CultureNotFoundException` for every culture except
the invariant one, so the server crashed in
`ResourceManager.LoadDefaultText`.
`ResourceManager` now:
- keeps the locale id of each translation table as a string, and falls
back to its language subtag to find another region of the same language;
- when the runtime cannot create the culture, parses a locale id that
matches the grammar of RFC 5646 §2.1 into the recommended casing of its
subtags. It still rejects a language without a region, and rejects a tag
that does not match the grammar, as before;
- caches the resolved locale ids, so loading the default texts creates
each culture once.
`docs/NativeAoT.md` no longer warns against invariant mode for servers.
The new tests (`ResourceManagerInvariantGlobalizationTests`) reproduce
both invariant-mode behaviors of the runtime: no cultures at all, and
cultures that carry invariant data. They also cover malformed tags, and
well-formed tags with extended language, variant, extension, and private
use subtags.
The branch includes a merge of master: #4707 moved translation lookups
to a lock-free snapshot of the tables, and the snapshot now copies the
locale-id-based tables.
### Validation
| Command | Result |
| --- | --- |
| `dotnet test tests/Opc.Ua.Server.Tests -f net10.0
-p:CustomTestTarget=net10.0 --filter
"FullyQualifiedName~ResourceManager"` | 100 passed, including the 39
tests of the new fixture. Before the fix, 10 of its first 22 tests
failed; 7 of the 17 grammar cases failed against the first version of
the parser |
| `dotnet test tests/Opc.Ua.Server.Tests -f net48
-p:CustomTestTarget=net48 --filter "FullyQualifiedName~ResourceManager"`
| 100 passed |
| `dotnet test tests/Opc.Ua.Server.Tests -f net10.0
-p:CustomTestTarget=net10.0`, on
|
||
|
Marc SchierandCopilot
|
e80d0ca9ac |
Use a 4-byte KeyNonce in the reference PubSub sample (#4710)
# Description
The first PubSub exchange in
[docs/PubSub.md](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/PubSub.md#run-a-first-publisher-and-subscriber)
delivers no DataSets on master or in the 2.0.0 source. The subscriber
only logs `DataSetReader Reader 1 faulted on MessageReceiveTimeout`,
while the publisher logs `UADP security wrap failed; dropping message`
with `ArgumentException: AES-CTR nonce must be exactly 12 bytes` for
every NetworkMessage.
Cause: the reference PubSub sample's demo keys use a 12-byte KeyNonce.
Since #4573, the AES-CTR counter block follows OPC 10000-14 Table 157
(`KeyNonce[4] || MessageNonce[8] || BlockCounter[4]`). With the sample's
keys, the publisher builds a 20-byte nonce, and `AesCtrTransform`
rejects it. The 2.0.0 release (release/2.0 at
|
||
|
Marc SchierandCopilot
|
0af7ae3a23 |
Update the guides and the MCP server docs for the 2.0.0 release (#4713)
# Description The packages on nuget.org are stable 2.0.0 now. The guides still told readers to use `--prerelease` and preview-only versions, and kept caveats that applied only to 2.0.0-preview.5 and earlier previews. Getting started, Your first information model, and Source-generated data types: - Add the packages without `--prerelease` and name 2.0.0 as the verified version. - With 2.0.0, a client with no server reports `BadNotConnected`, so the troubleshooting row drops the preview.5 `SocketException`. - 2.0.0 generates code for methods that declare their arguments inline, so the tutorial drops the preview.5 workaround note and its troubleshooting row, and says both forms are possible. MCP server (`docs/McpServer.md` and the tool's NuGet README `tools/Opc.Ua.Mcp/McpREADME.md`): - Install and run commands use the stable tool: `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp`, `dnx …`, and `dotnet tool install --global …`, without `--prerelease` or `2.0.0-preview.*`. - The stable tool bundles `Opc.Ua.Mcp.Robotics` and `Opc.Ua.Mcp.Vision`, so the note that `vision` and `vision,robotics` need the current source is removed. The docs now state that only the standalone `Mcp.Robotics` and `Mcp.Vision` libraries are preview packages (`2.0.0-preview.200`). - Not changed: `tools/Opc.Ua.Mcp/.mcp/server.json` still declares `"version": "1.0.0"` and lists only the original profiles in its description. ### Validation | Command | Result | | --- | --- | | Getting started Route B, the first-model tutorial, and every troubleshooting row, run exactly as documented against the 2.0.0 packages | Output matches the guides; 0 warnings | | Native AOT publish of the Route B server and client | 0 warnings; the native executables completed the trust flow and connected | | `dotnet tool exec OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dnx OPCFoundation.NetStandard.Opc.Ua.Mcp -- --version`, `dotnet tool exec "OPCFoundation.NetStandard.Opc.Ua.Mcp@2.0.*" -- --version` | `2.0.0+bc5da1006…` for each | | `dotnet tool install` of `OPCFoundation.NetStandard.Opc.Ua.Mcp` without `--prerelease` (into a temporary tool path) | Installs 2.0.0 | | `dotnet tool exec` of an uncached version with stdin redirected, as an MCP client launches it | Runs without a confirmation prompt | | MCP stdio `initialize` + `tools/list` against the 2.0.0 tool with `--profile vision,robotics`, `vision`, `robotics`, `full` | 64, 26, 42, and 138 tools | | Markdown link, anchor, contents, and fence checks over `docs/` | 0 issues | ## Related Issues - No tracking issue. Follow-up to #4555 after the 2.0.0 release. ## Checklist - [ ] 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. - [ ] 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. - [ ] 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> |
||
|
romanettandClaude Opus 5.5
|
36194f6a02 |
Avoid boxing and per-value copies on encoder, enum and server hot paths (#4715)
## Summary Allocation audit of the per-message / per-value paths (binary codec, built-in types, server request path, monitored items, client notification dispatch) looking for boxing and avoidable per-value copies. Measured with `GC.GetAllocatedBytesForCurrentThread` probe loops on net10.0 **and net48**, and with `dotnet-trace` allocation traces of the server load harness (#4707) for read/write/browse/subscription, server and client side — a value type in the "allocated types" list is a box. Wire output and error behaviour are unchanged. ### Fixed | Where | Cause | Frequency | Before → after | |---|---|---|---| | `Polyfills/System.IO.cs` (net48/netstandard2.0) | `BinaryWriter.Write(ReadOnlySpan<byte>)` polyfill copied with `ToArray()` — every string, array, ByteString, Guid write | per value | 32–64 B → 0; BrowseResponse×50 encode 15.3 → 10.9 KB | | `EnumHelper` (net48/netstandard2.0) | `EnumToInt32`/`Int32ToEnum`/`EnumToInt64` boxed outside `NET8_0_OR_GREATER` (every enum field, `Variant.From<TEnum>`, `EnumValue.From`) | per value | 24 B → 0 | | `BinaryDecoder.ReadString` + stream raw reads (net48) | temp `byte[]` per string, `BinaryReader.ReadBytes` | per value | BrowseResponse×50 decode 15.7 → 11.3 KB | | `WriteGuid` / `ReadGuid` (all) | `ToByteArray()` / `byte[16]` | per value (also Guid NodeIds) | 40 B → 0 each | | `VariantHelper.TryCastTo<TEnum>` (`DataValue.GetValue<TEnum>`), `EventRecordFieldReaders.GetEnum` | `Enum.ToObject` box | per call / per event field | 24 B → 0 | | `NodeManagerServiceDispatcher.GetManagerHandleAsync` | interface enumerator per validated node | per node | Write-100: 151.8 → 148.5 KB/request | ### Cold paths (third commit) - `Server.Auditing` write handler: `Convert.ToBoolean(object)` got a boxed `Variant` (not `IConvertible`) and threw `InvalidCastException` on any in-process write; now takes the boolean, `BadTypeMismatch` otherwise (new test). - `UpdateServerServiceLevel`: `Convert.ToByte(byte, IFormatProvider)` bound to the object overload. - Client publish-state / recovery-policy `HasFlag` (boxes on net48) → `(x & F) == F`. - `SequenceReader` big-endian float on net48/netstandard2.0 used `Convert.ToSingle(int)` (numeric conversion) instead of reinterpreting the bits. - JSON enum strings: one `Enum.GetName` + `string.Concat` instead of `IsDefined` + boxing `CoreUtils.Format`; same output (new test for the `EnumValue` overload). Left as is: `SamplingGroup` start-up `Convert.ToInt32(double, provider)` (rewritten by #4707), `SecureChannelRenewed` formatting (transport files in flight on the client perf branch), `Utils.IsEqual<T>` object path for structs (cold callers; changing it would mix `CompareTo` and `Equals` semantics), Machinery `HasFlag` (startup config). ### Not changed here - `BinaryEncoder.GetNodeIdEncoding` `Convert.ToUInt32(object)` / `Convert.ToByte(enum)` (72 B per NodeId encode, largest single item) and `ChannelAsyncOperation.Complete` boxing the `int` result are handled on the client performance branch. - By design: `Variant` / `NodeId` store `ArrayOf<T>`, Guid, ByteString identifiers boxed (`Variant.Copy` re-boxes RolePermissions during permission checks). ## Tests - New `BinaryCodecAllocationTests`: round trips in buffer and stream decoder mode, truncation errors, allocation per call (< 1 B/call over 20k calls; the counter can be off by an allocation quantum when other tests GC). - `Enum.ToObject` equivalence for `TryCastTo` and `EventRecordFieldReaders.GetEnum` across all underlying types. - net10.0 (third commit): Types 11465, affected Server 87, Client subscription 642 passed; net48 Types 11458 passed, `UA.slnx` 0 warnings. - net10.0: Types 11464, Core.Encoders 2947, Server 8386 passed; Core 6197 passed, 1 unrelated (`ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly`, passes alone). - net48: Types 11457, Core.Encoders 2947, EventRecordFieldReaders 25 passed; `UA.slnx` builds with 0 warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
28f1401324 |
Server: cut request-path lock contention and allocation; add load harness (#4707)
Profiling the reference server under load showed most request-path CPU spent spinning on contended locks, plus several avoidable allocations per request or per node. This PR removes them, switches the reference server to Server GC, and adds the load harness used to measure it. ## Results Paired A/B against `master` (3 alternating rounds, medians, 20 sessions, workstation GC on both sides, 24-core machine): | Scenario | Throughput | p50 | p90 | Server CPU / op | Server alloc / op | Lock contention | |---|---|---|---|---|---|---| | Read, 1 value | **+45%** (94k → 137k req/s) | −20% | −46% | −37% | −39% | −94% | | Read, 100 values | **+40%** | −26% | −17% | −24% | −25% | +130% ¹ | | Read, 100 values, SignAndEncrypt | **+48%** | −36% | −20% | −30% | −26% | +106% ¹ | | Browse (Server object) | **+33%** | −31% | −12% | −13% | −14% | −40% | | Write, 100 values | +13% | −12% | −4% | −10% | −19% | −23% | | Data-change subscriptions | ±0 ² | ≈ | ≈ | −14% | −17% | −29% | ¹ The faster server spends more time in the remaining per-node locks. ² Throughput is bounded by the writer rate in this scenario. Separately, Server GC instead of workstation GC doubles Read-100 throughput (21k → 44k req/s) and cuts p90 from 2.1 ms to 0.6 ms, because workstation GC paused the server for about half the wall time under load. The working set grows from ~210 MB to ~430 MB. ## Changes - **`ObjectPool<T>`**: lock-free slot array. `ConcurrentBag.Count` took every per-thread lock on each `Return` of the pooled response value-task source, once per request. - **`FairRequestQueue`**: - drops redundant gate entries: the fail-fast pre-checks and the stopped check read volatile fields and are re-checked under the gate; the owner count is folded into selection; - coalesces wake pulses with a pending flag, so the bounded signal channel's lock is only taken when no token is outstanding. Wake-count semantics in the tests are unchanged. - **`DefaultServerResourceIsolationProvider`**: - compares peer addresses without `GetAddressBytes()` allocations; - builds the unsecured verification context only when a classifier is configured. - **TCP listener / channel**: caches the certificate DER copies per channel, keyed on the certificate instance. `X509Certificate2.RawData` returns a fresh copy on every access, and the listener read it for every request. - **`EndpointBase`**: resolves the request `ActivitySource` once instead of `Assembly.GetCallingAssembly()` per request. - **Node managers**: - one `ServerSystemContext` copy per request (cached on the `OperationContext`) for per-node callbacks: permission metadata, node metadata, view membership, method lookup; - removes a discarded copy in `GetReferenceDescriptionAsync`; - per-request operation caches are plain `Dictionary` instead of a `ConcurrentDictionary` with 24 lock objects; - the validation cache is built in one pass; - fixes zero-capacity error lists. - **`ResourceManager`**: translation lookups read an immutable snapshot that is rebuilt after writes. Browse serialised on the global lock for every display name. - **`MonitoredItem`**: shares the default `DataChangeFilter` instead of allocating one per sampled value. - **Reference server**: `ServerGarbageCollection=true`; new "Garbage collector mode" section in `docs/ServerScalability.md`. - **`perf/`**: `ServerLoadHarness` (out-of-process server plus load driver, server-side CPU/GC/alloc/contention deltas, optional `dotnet-trace` capture), `TraceAnalyzer` (TraceEvent-based CPU/allocation/contention aggregation), and paired A/B scripts. Not part of `UA.slnx`. ### Behaviour notes for custom node managers - The `IDictionary<NodeId, NodeState>` operation cache handed to overrides is now a `Dictionary`. It was a `ConcurrentDictionary`, but only one request flow uses it. - Within one request, per-node callbacks of a node manager share one `ServerSystemContext` copy. ### Later commits on this PR - **Permission checks without `NodeMetadata`** (`9068684c3`): the role-permission and access-restriction validators run on an internal `PermissionMetadata` struct; `AsyncCustomNodeManager` fills it on the stack from the same attributes and namespace defaults. Browse's reference filter reads 3 attributes per target instead of 13. Read 100 values: 120.6 → 95.7 KB/request, ~21k → 26k req/s; Browse 67.6 → 51.8 KB, 26.7k → 29.8k req/s. - **Read-only channel evidence** (`936b28d76`, `dc638ab9f`): `SecureChannelContext.ClientChannelCertificate`, `ServerChannelCertificate` and `ChannelThumbprint` are `ByteString`, so the cached per-channel certificate buffer is shared without a copy and cannot be altered by handlers or classifiers. **Public API change**, see the new section in `docs/MigrationGuide.md`. The isolation provider's revalidation snapshot still copies. - **Session lookup** (`936b28d76`): `SessionManager.TryGetSessionContext` takes the candidate snapshot without the bindings lock; the validating check under the lock is unchanged (2 instead of 4 global lock acquisitions per request). - Unit tests for `ObjectPool`, the per-request system context cache, the translation snapshot, `SecureChannelContext` null semantics and a stopped `FairRequestQueue`; `perf/` follows the repository conventions. ## Testing - `Opc.Ua.Server.Tests` (net10.0): 8386 passed before the `ByteString` change. After it, 13 secure-session and isolation tests failed; both causes are fixed in `dc638ab9f` and those tests (25 with their variants) pass. A full rerun of the Server, Sessions, Core, Redundancy, LDS and GDS suites on the final commit is in progress. - Full solution builds for net10.0 and net48; `dotnet format whitespace` clean. ### o6-automation opcua-benchmarks (sampling, deep pipelines) Replicated both tests of the [o6-automation benchmark](https://github.com/o6-automation/opcua-benchmarks) in the harness (`--scenario counters`, and `read` with 10 clients × N outstanding), paired against `origin/master`: | Test | master | this PR | |---|---|---| | Subscription sampling, 100 Hz, every item ≥ 99 % of samples, 1 item | FAIL (75–94 %) | PASS (100 %) | | same, 1024 items | FAIL (64–73 %) | PASS (99.9 %) | | same, capacity | — | between 4096 and 6144 items (~410–520k samples/s) | | Read, 10 clients × 100 outstanding, None | stalls (0–1.7k req/s) | 121–131k req/s | | Read, 10 clients × 100 outstanding, SignAndEncrypt | stalls (0 req/s) | 89–91k req/s | | Read, 10 clients × 1000 outstanding | channels faulted (`BadTcpNotEnoughResources`) | 0 errors, ~45–54k req/s | - **Fixed-rate sampling** (`30b769d13`): the sampling loop waited one interval after each sample, adding timer overshoot and sampling time to every cycle; samples now have absolute deadlines. - **Async response send** (`cdac5c5ca`): `SendResponse` blocked thread-pool workers on the channel gate; with many requests in flight the receive loop that was handed the gate waited for a pool thread and the channel stalled. The TCP, Kestrel TCP and WebSocket listeners now use `SendResponseAsync`. - **Backpressure instead of channel fault** (`191cd2ba7`): server channels stop reading while more than 100 responses are queued and resume at 50, instead of faulting the channel. Client channels keep reading. At 10,000 outstanding requests throughput is ~50k req/s with or without the limit engaged, so the remaining gap to the 100-per-client case is not the backpressure. ## Not changed - `SentMessageQueue` deep-clones every `NotificationMessage`. Retained messages return their pooled payloads on acknowledgement while the response copy may still be queued for send, and a client can acknowledge a sequence number before receiving it; the clone keeps a recycled payload from reaching another session's response. Removing it needs a send-completion hook from the transport. - Generated responses allocate a default `ResponseHeader` that is replaced (~2-3 % of per-request allocation). Fixing it means making structure fields of every generated type lazily initialized, a generator-wide change to default values, equality and encoding. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
romanettandClaude Opus 5.5
|
1fb1134974 |
Tests: scope MeterListeners to the meter under test (fix flaky ClientBaseTests metrics test) (#4708)
## Problem `ClientBaseTests.ActivityTraceFlags_Metrics_ShouldRecordMetricsOnly` failed intermittently in full-suite runs: ``` Expected: "opc.ua.client.request.duration" But was: "opc.ua.channel.reconnect.duration" ``` `TelemetryContextBase.CreateMeter()` returns a new `Meter` per call, named after the calling assembly, so `ClientBase`'s meter and `ClientChannelManagerMetrics`' meter are both named `Opc.Ua.Core`. `TestMeterListener` enabled every instrument whose `Meter.Name` matched, so reconnect-duration measurements from `[Parallelizable]` channel manager fixtures were recorded and could precede the request duration. ## Fix (test-only) - `ClientBaseTests.TestMeterListener`: enable instruments only when `ReferenceEquals(instrument.Meter, meter)`, i.e. the meter instance the client under test created. Assertions unchanged. - `CryptoProviderModelTests.AuditorPublishesAProviderMetric`: same flaw — it enabled every `opc.ua.crypto*` instrument process-wide (fixture is `ParallelScope.All`), so another auditor's uncertified gauge could overwrite the observed count. Now scoped to the auditor's own meter (read via reflection, as `ClientBaseTests` already does for `ClientBase`). Other `MeterListener` users reviewed and left as-is: `NodeCacheTests` / `AdapterMetricsTests` are non-parallel fixtures; channel manager listeners only assert existence and filter by endpoint tag. ## Verification - `ClientBaseTests` + `CryptoProviderModelTests`: 4× consecutive, 25/25. - `FullyQualifiedName~Opc.Ua.Core.Tests.Stack` (net10.0): 2 clean passes, 3548 passed / 80 skipped. One attempt hung in unrelated `ClientChannelManagerManagedTests` async tests (`ReconnectAsyncNotifiesAttachedParticipantsAsync` / `ReleasingOneLeaseKeepsChannelAliveForOtherAsync`) and was rerun — a separate intermittent hang. - net48 build of `Opc.Ua.Core.Tests`: 0 warnings, 0 errors; `dotnet format whitespace` clean. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
