3 Commits

Author	SHA1	Message	Date
romanett	ea93c86da2	Fix the Opc.Ua.Client defects reported by the client stack audit (#4471) ... # Description Fixes the 78 `Opc.Ua.Client` findings of a read-only bug audit of `Opc.Ua.Client` / `Opc.Ua.Types` / `Opc.Ua.SourceGeneration` taken at a05a36055, plus the regressions a subsequent review of those fixes turned up. Every behavioural change was checked against the OPC UA specification, and the citations are in the code comments. ## Session, reconnect and failover - The reconnect `finally` re-acquired `m_reconnectLock` with the caller's token, so a cancelled reconnect left the session stuck in `Reconnecting` forever and every later reconnect failed. It now uses `CancellationToken.None`. - An in-place session recreate tore down subscriptions it should not have: `transferred` is all-or-nothing, so a single failed transfer reset and recreated the ones the server had just taken over, orphaning them on the new session. `TransferSubscriptions` now reports the per-subscription outcome and only the subscriptions the server did not take over are recreated. A token-reuse failover keeps the same server session, so nothing is transferred or reset there at all. - In-place recreate only tears down the channel it created itself; a caller-supplied channel or a channel-manager lease is left to its owner. - `Snapshot(out SessionState)` went through the `SessionOptions` record copy constructor, which drops every member declared on `SessionState`, so the paired `Snapshot`/`Restore` API could not round-trip. - The user-token policy override restored the policy URI on failure but had already disposed the ECC ephemeral key the previous identity needs, so the identity the rollback claims to preserve could no longer be reactivated. The key is now parked and restored. - `SetTriggering` batching sliced the remove list past its end when a call had more adds than removes (Part 5 §6.3.11 bounds the *sum* of both arrays, so an individual list may be shorter than the limit). - The certificate-less `None` attempt dropped the request header, and with it the ECDH ephemeral key that travels in `additionalHeader` (Part 4 §7.1/§7.15). - `Session.Equals` compared a `ConfiguredEndpoint` against an `EndpointDescription`, so a session did not equal a session on the same endpoint. ## Managed session, pool and connection state machine - `ManagedSession.Dispose()` blocked on the connection state machine's worker, which raises `ConnectionStateChanged` synchronously — disposing from that handler deadlocked permanently. The machine now recognises its own flow and completes the teardown as the worker's continuation. - `ManagedSessionFactory` dropped `updateBeforeConnect`, `connection` and `reverseConnectManager` on all four `CreateAsync` overloads. - The session pool ran its shared connect on no token at all, so once the only caller gave up nothing could abort it and `RemoveAsync`/`DisposeAsync` waited out the whole reconnect budget. Each entry now owns the token that aborts its connect, and removal closes *and* disposes the session. - An explicit `engineFactory` was silently ignored for `DefaultSessionFactory` subclasses, which quietly downgraded the session to the classic subscription engine. ## Subscriptions - Fixed a lock-order inversion between `m_subscriptionLock` and the partition locks in `CompleteAsync` and `DisposeAsync`. - `OwnsSubscriptionId` stopped at the first registered subscription carrying the id — but in the very case it guards, the asker still holds its own retired id, so the answer depended on `HashSet` enumeration order and the delete could still tear down a sibling that had been handed the recycled id. - `MessageProcessor`'s republish gap walk was unbounded; it is now bounded by the server's retransmission queue (Part 4 §5.14.5). - A publish acknowledgement is sent even when a notification handler throws — the ack means "the server may delete the message" (Part 4 §5.14.5.2), not "I processed it". - Loaded monitored items did not raise the process-wide client-handle counter, so a fresh process minted handles that collided with the loaded ones; a transfer-restored item was also stamped `BadOperationAbandoned` and left at the default monitoring mode. - `AutoSetQueueSize` was not idempotent, so the queue size ratcheted up on every subscription state change. - The snapshot dropped the partition and recovery options entirely, and clamped the sampling interval and the secondary-partition idle timeout to zero — turning the Part 4 §7.21 "use the publishing interval" sentinel and "never delete an idle partition" into something else. - The state stream is positional OPC UA binary and the reader cannot detect an absent field, so appending the partition options would have mis-decoded every previously saved file. The stream now carries a format version and the first format is decoded through a frozen copy of its shape. ## Browse, node cache and the remaining clients - Managed browse wrote a retry pass's status into whatever node happened to sit at that offset in the caller's list, and retried continuation-point errors without any bound. It now keeps the caller's slot and gives up on passes that make no progress — a fixed pass cap would cut short a large browse against a conforming server with a small continuation-point quota. - Continuation points are released when a consumer stops enumerating early, as Part 4 §5.9.3.2 requires. - The node cache reordered its batched results relative to the inputs, which mispairs them for every consumer that matches positionally; its supertype walks could also spin forever on a server whose `HasSubtype` chain loops back on itself. - A browse error was cached as "this node has no references"; it is now surfaced to the direct caller, while the bulk and hierarchy walks treat the node as having no references for that one operation without caching it, so a single un-browsable node cannot fail a whole batch. - `WebApiClient` mutated a caller-supplied (typically `IHttpClientFactory`-pooled) `HttpClient`, which throws once that client has sent its first request; those instances are configured per request instead. - `PathCache` was not thread-safe. ## Review follow-ups The last two commits are the result of reviewing the audit fixes themselves. Worth calling out: the `Task.WaitAsync(CancellationToken)` polyfill in `Opc.Ua.Types` never checked `IsCompleted`, so an already-finished task plus an already-cancelled token reported cancellation on every pre-net8.0 target where the BCL overload returns the result. It is now aligned with the BCL and pinned by a test. ## Related Issues - No tracking issue; this PR is the result of a read-only audit of the client stack. ## 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. ### Verification 35 regression tests were added under `tests/Opc.Ua.Client.Tests/AuditRegressions/` (category `AuditRegression`), one per fixed defect where the defect is observable at unit level; most were mutation-checked by reintroducing the bug and confirming the test fails. Locally on **net10.0**: `Opc.Ua.Client.Tests` 2252/2252, `Opc.Ua.Client.ComplexTypes.Tests` 2773/2773, `Opc.Ua.Bindings.Https.WebApi.Tests` 346/346, and all 23 library and test projects that reference `Opc.Ua.Client` build clean. `Opc.Ua.Client` also builds clean for **netstandard2.1**, the target that actually compiles the `WaitAsync` polyfill. The full **UA.slnx** solution and the .NET Framework targets have not been run locally — left to CI. One pre-existing failure in `Opc.Ua.Types.Tests` is unrelated to this branch: `WotSpecFixtureManifestTests.EveryEmbeddedExampleMatchesItsRecordedHash` (embedded WoT fixture hashes) fails identically with these changes reverted. ### Notes for reviewers - Two review findings were deliberately left unfixed and are documented in the code: `ManagedSession.CreateAsync` has to write the engine choice onto a caller-supplied `DefaultSessionFactory` subclass (the only alternative is changing the `ISessionFactory.Create` contract), and the connection state machine's "am I on my own worker" flag is an `AsyncLocal`, so it also flows into work a `StateChanged` handler spawns. The previous behaviour in that second case was a hard deadlock. - `Session.Dispose()`'s single `GetAwaiter().GetResult()` bridge to `CloseAsync` is left as is: `Dispose(bool)` must be synchronous and `DisposeAsync` already exists. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-14 08:22:28 +02:00
Marc Schier	7411654c8d	Complete OPC UA Part 11 Historical Access (#4390) ... # Description Completes the OPC UA Part 11 Historical Access work requested by #4387 and the historian framework integration requested by #4400. The conformance/evidence catalog covers all 37 released UACore 1.05 Historical Access profiles: 15 Server facets and 22 Client facets, together with the standard aggregate functions and their conformance units. Server facets are advertised only when the resolved providers expose their required interfaces and capabilities. ## Server and provider support - Completes raw, modified, at-time, processed, annotation, structured, and event history. - Dispatches best-effort, bulk, and transactional updates with aligned statuses, old values, diagnostics, and rollback state. - Supports mixed history-update detail types and batched structured/annotation operations. - Uses composite identities and exclusive cursors for same-timestamp structured, modified, and event values. - Captures live data and events, installs historical configuration objects, and emits typed audit payloads. - Derives aggregate monitoring filters from per-node historian capabilities and rejects unsupported processed aggregates before provider dispatch. - Supports tri-state `Historizing`: set, clear, or preserve provider-owned state. ## Client support - Exposes modified values together with `ModificationInfo`. - Adds event-history reads and event insert/replace/update/delete operations. - Adds structured and annotation batching with strict response validation. - Shares continuation handling and release-on-abandon across read families. - Accepts inherited standard event fields rooted at event subtypes. ## Review hardening - Registers DI-selected historians before node-manager reconciliation and capability publication, while preserving provider ownership. - Uses committed Raft read barriers for linearizable `TryGetAsync` and `ScanAsync`. - Keeps claimed continuation state immutable until a fresh successor is saved, restoring the original after transient failures. - Uses exact-incarnation compare-and-delete so delayed cleanup cannot remove a restored same-ID continuation. - Cleans up indeterminate initial and replacement saves, and propagates shutdown cancellation through cleanup resolution. - Preserves legacy continuation envelopes/cursors and distinct annotation parent/property identities. - Deletes every structured entry at a requested timestamp and uses the complete modified-history ordering tuple. - Shares native/fallback at-time and AnnotationCount calculations, including reverse endpoints and cross-framework fractional intervals. - Primes modified aggregate items before buffered live delivery and deduplicates history/live overlap using provider-specific identities. - Keeps committed Modify results successful while reporting priming failures through the monitored item's notification queue. - Keeps protected-notification state live-only for queues larger than one. Queue size one retains the newest notification without pinning errors or transferring priority during resizing. Durable restore retains ordinary definitions, last values/errors, and raw queues without recreating priority. - Isolates aggregate calculation, modification preparation, and historical/live handoff in `AggregationFilterHandler`, retaining existing provider/factory wiring and supported past-start behavior. The larger-queue protection policy is unchanged pending the open review discussion. - Uses one `IMonitoringFilter` owner for protocol filters or the aggregation handler, eliminating parallel effective-filter storage and retaining the current definition during preparation. The original client filter stays distinct. - Integrates current master, retaining custom monitored-item creation decisions and per-node history callbacks. ## Redundant-server alignment and samples - Adds a protected shared historian with immutable segments, manifest CAS publication, writer fencing, and restart recovery. - Persists portable continuation state before exposing it to clients and transfers ownership during mirrored-session takeover. - Rejects eventual-consistency and active/active multi-writer historian configurations rather than silently degrading. - ReferenceServer and ConsoleReferenceClient demonstrate discovery, paging, raw/modified/at-time/processed reads, annotation/structured CRUD, event capture, and event CRUD. - RedundantServer and RedundantClient demonstrate active/passive history, continuation recovery after replica termination, and post-promotion writes. - Updates Historical Access, High Availability, profile, migration, and sample documentation. ## Compatibility - New 2.0 collection boundaries use `ArrayOf<T>` and public byte payloads use `ByteString`. - Removes the legacy public virtual `SamplingGroupManager.CreateMonitoredItem` / `ModifyMonitoredItem` entry points and protected creation factory, as agreed in review. Migration guidance points to the monitored-item manager and construction-decision APIs. - Retains established persisted formats and current write layouts. Retired interim notification-state formats (sample v2 / shared v4) are rejected, as agreed in review. - Keeps the historian integration compatible with .NET Framework and NativeAOT. ## Validation ### Latest CI source repair `73c619435` forwards the existing cancellation token at both predefined-node creation calls in the PR-added History event fixture. The merged lifecycle overload exposed these two CA2016 errors, which blocked History jobs, the solution build matrix, and CodeQL on GitHub and Azure. The exact compiler failure was reproduced locally; the repaired fixture passes 4/4 integration cases on net10.0 and 4/4 on net48 with clean scoped diagnostics. No assertions, tests, coverage thresholds, or CI configuration were weakened. The repaired History jobs on Linux/macOS and all four Azure solution builds now pass. The Windows all-TFM GitHub job instead lost its hosted runner's connection; no build log was recoverable, and GitHub recorded a runner-loss annotation rather than a compiler/test diagnostic. One unchanged retry was requested. Remaining checks are monitored with `gh pr checks --watch --fail-fast --interval 30`; CI is not yet reported as green. ### Latest base integration `bd52e5fea` merges upstream `master` at `da7412c42`. The lifecycle conflict retains upstream's shared cancellation-aware registration helper and the historian behavior. The hosted-historian fixture forwards its existing token to the new lifecycle overload. GDS project renames and all three AOT test hosts are retained, with their project references resolving after the merge. | Integrated scope | net10.0 | net48 | | --- | --- | --- | | Server lifecycle, fluent authoring/import, historian, monitored-item and hosting regressions | 732 passed | 732 passed | | NodeState lifecycle regressions | 216 passed | 216 passed | | Automatically merged Raft client fixtures | 28 passed | 28 passed | No selected tests failed or were skipped. The manually resolved files pass scoped formatting/analyzer checks. One CA1861 warning remains in the incoming `NodeSetImportIntegrationTests.cs`; that file is byte-identical to upstream master and was not rewritten or suppressed during integration. ### Latest feedback follow-up `102954eaa` replaces split filter references with the shared `IMonitoringFilter` contract implemented by protocol filters and `AggregationFilterHandler`. Targeted server regressions pass **103/103 on net10.0 and 103/103 on net48**; scoped formatting/analyzers are clean, and the updated XML-documentation inventory has zero gaps across 188 non-generated PR C# files. `93c5b5db8` renames `AggregationFilterHandler`, makes it the owner of the server-revised aggregate filter while preserving the original client filter, and removes the retired interim notification-state readers. Server regressions pass **101/101** and shared-store regressions **64/64**, on both net10.0 and net48, with no failures or skips. Runtime-file formatting/analyzers are clean. `5ccf45630` completes the public/internal XML-documentation sweep. The Roslyn inventory finds zero undocumented eligible declarations across all 186 non-generated PR C# files; the 110-file documentation slice changes only XML comments and spacing. `f17765475` applies the three requested Markdown edits. ### Earlier feedback follow-ups `a53950737` removes single-slot notification pinning and extracts aggregate implementation from MonitoredItem. Targeted sampling, aggregate modification, history/live handoff, queue/lifecycle, restore, and serialization cases pass **101/101 on net10.0 and 101/101 on net48**, without failures or skips. Scoped formatting/analyzers are clean; the focused review found no significant issues. The final formatted monitored-item fixtures also pass 45/45 on net48. The preceding `b21555568` removes the legacy sampling API bridge and durable notification-priority state. Its targeted Server cases passed 97/97 and shared subscription-store cases passed 64/64 on each framework. The broader evidence below remains pinned to its original tested commit, not attributed to these follow-ups. ### Earlier integrated-implementation validation Validated implementation commit: **`3efae597f`**. The following commit, `f6dbefb1c`, only adds two explanatory comment lines for the final standards review; it changes no executable code. | Scope | net10.0 | net48 | | --- | --- | --- | | Server | 5,187 passed; 56 skipped | Affected surface: 2,160 passed; 1 reproduced baseline failure; 56 skipped | | Client | 2,202 passed; 2 skipped | 2,204 passed; 5 skipped | | History integration | 519 passed; 21 skipped | 519 passed; 21 skipped | | Redundancy server | 641 passed | 641 passed | | Redundancy client | 124 passed | 124 passed | | Core shared-store regression suite | 23 passed | 23 passed | All six affected samples passed non-incremental Release/net10.0 builds with zero warnings and errors: ConsoleReferenceClient, ConsoleReferenceServer, RedundantServer, RedundantClient, MinimalBoilerServer, and PumpDeviceIntegrationServer. The published self-contained win-x64 NativeAOT executable is 116,924,928 bytes and has no CLR header or managed runtime dependency. Direct execution passed **HistoryAotTests 3/3** and **RaftAotTests 2/2**. Final standards and specification reviews found no confirmed outstanding high/medium functional gaps or hard standards violations. The remaining constructor-purpose clarification is addressed in `f6dbefb1c`. ### Reproduced baseline exceptions The net48 hosted-startup timeout also reproduces in a pristine worktree at the previous PR head, `24cf1416d`, in `RegisterPostStartRegistriesWiresHistorianAndAliasStoresOnPlainServerAsync`, with the same subsequent certificate-leak assertion. The corresponding renamed case fails before startup in the reviewed branch as well. This is reported separately from the net48 affected-surface results, not counted as a passing full Server suite. The net48 matrix selected the historian, aggregate, monitored-item, fluent, and continuation surfaces and excluded the Hosting namespace. `DurableDataValueQueueVerifyReferenceBatchingAsync` failed at item 1000 in the net48 affected-surface run and passed its single isolated rerun. The same failure at the same assertion reproduces on pristine `849e16acc`; the test, sample queue, queue factory, and batch persistor are unchanged. The original failure remains in the table rather than being erased by the rerun. ### CI source repairs - `4be7eb8f7` keeps diagnostics/configuration managers out of distributed address-space replication. Those managers inherit the source interface, but their live server state and redundancy metadata must remain replica-local. The ownership regression passes on net10.0 and net48, and all nine redundancy sample CI tests pass locally. - `d17f467b3` completes the sample startup repair: core managers are also replica-local; the startup banner no longer advertises readiness before initialization; the producer starts after distributed services; and native Raft startup no longer forces disruptive extra elections. The full sample suite passed 9/9, with initialized `RedundancySupport=Hot`, and focused Raft/address-space regressions passed 8/8 and 3/3. - `81b0b72be` moves the unchanged distributed-historian NativeAOT scenario into a smaller, always-run companion executable to bound Apple's linker input. All 137 AOT tests remain present; the moved test and helpers are unchanged. GitHub and Azure publish/run both native executables, with separate result directories where they share a job. The companion passed actual Windows NativeAOT execution. Linux and Intel macOS CI passed both executables, but this split alone did not fix ARM64 macOS linking. The remaining historian graph is rooted by ReferenceServer startup, not just the relocated test. No test exclusion or speculative linker setting was added to conceal it. - `8a5842913` isolates the independent MCP test graph into an always-run native host. The existing MCP source and assembly leak hooks are linked unchanged, and evaluated compiler inputs prove exactly one owner for every test source: 133 main + 3 MCP + 1 historian = 137 declarations. All three hosts run on every AOT platform. The main Windows native object is 9.35% smaller; both changed publishes had zero diagnostics. MCP passed 3/3; the identical main executable passed 133/133 on one unfiltered followup after two initial endpoint-discovery connection failures, which remain recorded in the local evidence. - The previous Intel macOS job successfully published and executed its native tests, then failed on artifact-upload DNS (`ENOTFOUND`). No upload gate, shared environment setting, or cache configuration was disabled. The first repair round also encountered the unrelated `MetricsAreEmittedForChannelLifetimeAsync` timing failure on macOS Core. Neither that test nor channel-manager code is changed by this PR, and its assertions remain intact. It passed unchanged on the final repair head. On `d17f467b3`, the Linux and macOS redundancy sample jobs pass; the remaining failure was ARM64 Apple's `too many large addends` assertion. The verified upstream relocation-anchor fix (dotnet/runtime#124721, backport dotnet/runtime#132171) is not present in the shipping ILCompiler 10.0.11. An experimental multi-module probe failed and was rejected, not added to CI. On `8a5842913`, [`aot-macos-latest`](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/34102158680/job/101678934057) successfully publishes and executes all three native hosts on ARM64: 136 main runtime cases, 1 historian case, and 3 MCP cases, with no skips. The previous Apple linker blocker is resolved. Linux NativeAOT also passes. No assertion, test, shared environment setting, cache, or required gate was weakened. The same CI run reports a separate macOS Sessions teardown failure: all 662 executed tests pass, but the global leak assertion detects one undisposed certificate. The MCP partition changes no runtime, Sessions, or test-fixture code. A single unchanged diagnostic rerun is queued once GitHub permits rerunning that job; the failure is not suppressed or counted as green. Other CI checks are still pending. ## Related issues - Fixes #4387 - Fixes #4400 ## Checklist - [ ] I have signed the CLA and read the contributing guide. - [x] I have added regression tests and directly related documentation. - [x] I have retained working reference and redundant-server sample wiring. - [x] Final validation and review evidence is complete, including the baseline exceptions above. - [ ] All required CI checks pass; the fail-fast rerun for `73c619435` is pending. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9dbd9527-d818-47a0-984c-fbb1472d3d6b	2026-09-08 11:32:23 +02:00
Copilot	d525bc3f28	Bound reconnect cleanup and organize samples by domain (#4276) ... # Description This PR fixes bounded ManagedSession reconnect cleanup and reorganizes the sample applications into domain-focused folders. ## Reconnect cleanup - Bounds reconnect integration cleanup with independent 30-second close and dispose limits. - Sends `CloseSession` through the awaited transport request path during `Session.DisposeAsync`. - Preserves explicit `Session.CloseAsync` behavior that waits for or cancels outstanding publish requests. - Covers the disposal request path with a regression test. ## Sample organization - `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer` - `samples/Redundancy`: `RedundantClient`, `RedundantServer`, `RedundantPubSub` - `samples/Isa95`: `MinimalIsa95Server` - `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`, `MinimalClient` - `samples/DI`: `PumpDeviceIntegrationServer` - `samples/Lds`: `ConsoleLdsServer` - `samples/Reference`: `ConsoleReferenceClient`, `ConsoleReferenceServer` - `samples/PubSub`: `ConsoleReferencePubSubClient` `UA.slnx` now exposes the same requested domain groups. Project references, tests, runtime sample probes, CI workflows, Dockerfiles, Compose files, helper scripts, and documentation were updated to the new paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`. ## Validation - `dotnet build UA.slnx` - 3 Redundancy short-haul sample tests - 70 DI sample tests - 914 OpenUSD tests - 1 WotCon model-materialization test - 137 AOT tests - `docker compose config` for all 6 moved Redundancy Compose files ## Related Issues - Fixes #4273 ## 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. - [x] I built the complete `UA.slnx` solution locally. - [ ] I ran all tests locally against at least .NET Framework and .NET 10. - [ ] 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-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636	2026-08-21 04:35:47 +00:00