mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-22 09:37:12 +08:00
fuzzing
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
0fbc900919 |
Add opt-in SubscriptionRecoveryPolicy for unsolicited Good_SubscriptionTransferred (#3540) (#3847)
# Description Adds an opt-in `SubscriptionRecoveryPolicy` for both the Classic V1 and V2 subscription engines that lets the client recover from server-emitted `Good_SubscriptionTransferred` notifications which arrive against a freshly created subscription (observed against Kepware after a server re-initialise, very likely a leak from the pre-restart session's pending status notifications onto the new session — abetted by Kepware re-using `SubscriptionId = 1` across generations). Per [OPC UA Part 4 §5.14.7](https://reference.opcfoundation.org/Core/Part4/v105/docs/5.14.7), `Good_SubscriptionTransferred` is supposed to be delivered to the *old* Session whose subscription was transferred away via `TransferSubscriptions`. The existing V1 behaviour of stopping the per-subscription publish loop on receipt is therefore **spec-compliant**; the issue is that some servers deliver the notification on a Session that is still actively using the subscription, leaving the per-subscription dispatch silently disabled. The fix is **opt-in** and **non-breaking**: - New `SubscriptionRecoveryPolicy` enum (`ReportOnly` default, `RecreateOnUnsolicitedTransfer`). - V1 Classic `Subscription.RecoveryPolicy` property; V2 `Subscriptions.SubscriptionOptions.RecoveryPolicy` init property. - When the policy is `RecreateOnUnsolicitedTransfer`, the SDK drops queued acks for the dead subscription id (avoiding `BadSubscriptionIdInvalid` ack errors on servers that re-use ids) and recreates the subscription in place on the same Session. ## What changed **Public API additions (non-breaking, defaults preserve current behaviour):** - `SubscriptionRecoveryPolicy` enum (`ReportOnly`, `RecreateOnUnsolicitedTransfer`). - V1 `Subscription.RecoveryPolicy` property. - V2 `Subscriptions.SubscriptionOptions.RecoveryPolicy` init property. - Internal `IMessageAckQueue.DropPendingForSubscription(uint)` and protected `MessageProcessor.AckQueue` accessor. **V1 Classic — `Libraries/Opc.Ua.Client/Subscription/Classic/Subscription.cs`:** - Replaces the inline `Good_SubscriptionTransferred` branch with `HandleGoodSubscriptionTransferred`. `ReportOnly` preserves the original behaviour (stop publish timer/worker); `RecreateOnUnsolicitedTransfer` triggers `TriggerUnsolicitedTransferRecovery`, guarded by `m_recreateAfterTransferInProgress` for idempotency. - Recovery drains pending acks via the new `ClassicSubscriptionEngine.RemoveAcknowledgementsForSubscription` **before** resetting the subscription, then runs `DeleteSubscription()` + `CreateAsync()` on the same session so the publish worker restarts via `StartKeepAliveTimer`. - Discriminator (`Session != null && !Session.Reconnecting && Created`) defers to the reconnect pipeline if it is already running. **V2 — `Libraries/Opc.Ua.Client/Subscription/Subscription.cs`:** - Completes the `OnStatusChangeNotificationAsync` TODO: now always fires `OnPublishStateChanged(mask)` so `PublishState.Transferred` actually reaches handlers (previously the no-op override silently swallowed the flag). - Under `RecreateOnUnsolicitedTransfer`, calls `AckQueue.DropPendingForSubscription(deadId)` then existing `ResetToRecreateAsync` on a worker task — no service calls from the dispatch path. - V2 `SubscriptionManager` implements `DropPendingForSubscription` via drain-and-refill on the prioritised ack channel. **Tests — `Tests/Opc.Ua.Client.Tests/Subscription`:** - V1 in `Classic/SubscriptionUnitTests.cs`: `ReportOnlyPolicyDoesNotRecreateOnUnsolicitedTransferAsync`, `RecreatePolicyRecreatesSubscriptionOnUnsolicitedTransferAsync`, `RecreatePolicyCollapsesConcurrentNotificationsAsync`, `RecreatePolicyDoesNotRecreateWhileSessionReconnectingAsync`. - V2 in `SubscriptionTests.cs`: `GoodSubscriptionTransferredRaisesPublishStateTransferredAsync`, `GoodSubscriptionTransferredRecreatesSubscriptionWhenPolicyEnabledAsync`, `GoodSubscriptionTransferredCollapsesConcurrentRecreatesAsync`. - `FakeMessageAckQueue` gained a `DroppedSubscriptions` recorder so the V2 test can assert ack pruning ran. **Docs — `Docs/TransferSubscription.md`:** - New "Recovering from unsolicited `Good_SubscriptionTransferred`" section with spec context, usage examples for both engines, and the caveat that recovery is recreate-based — **not** lossless failover; the server-side retransmission queue and triggering relationships tied to the invalidated subscription id are not preserved. ## Validation | Scope | Result | | --- | --- | | `Libraries/Opc.Ua.Client` build (`net10.0`, `net48`) | 0 warnings, 0 errors | | `Tests/Opc.Ua.Client.Tests` build (`net10.0`, `net472`) | 0 warnings, 0 errors | | New tests on `net10.0` and `net472` | 7/7 pass on each | | Full subscription unit-test suite (`SubscriptionUnitTests` + `SubscriptionTests` + `MessageProcessorTests` + `SubscriptionManagerTests`) | 110/110 pass on each TFM | | Entire `Opc.Ua.Client.Tests` suite (`net10.0`) | 1268/1268 pass | | Classic transfer integration tests (`Tests/Opc.Ua.Subscriptions.Classic.Tests`, `net10.0`) | 20/20 pass | | V2 transfer compliance tests (`Tests/Opc.Ua.Subscriptions.Tests`, `net10.0`) | 9/9 pass | > Marked as **draft** because the maintainer wanted to first confirm whether the stack is spec-compliant before deciding which knobs to ship; this PR documents the investigation (stack is compliant), and surfaces an opt-in recovery policy that addresses the field symptom without changing the default behaviour. Convert to ready-for-review once the design intent is acknowledged. ## Related Issues - Fixes #3540 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [ ] I have addressed **all** PR feedback received. --------- Co-authored-by: marcschier <marc.schier@github.com> |
||
|
|
dbc09b8e22 |
Adopt System.TimeProvider across the stack (fixes #2458) (#3829)
# Description Adopt [`System.TimeProvider`](https://learn.microsoft.com/dotnet/api/system.timeprovider) across the stack so that every place that creates timers, runs duration math, or schedules periodic work is mockable, deterministic in tests, and immune to wall-clock changes. The migration touches Core, Core.Types, Server, Client, PubSub, Configuration, Gds (server + client), Lds, and the Quickstarts/Console reference apps. Per-component changes follow the same pattern: 1. Add a `private readonly TimeProvider m_timeProvider;` field. 2. Accept a nullable `TimeProvider? timeProvider = null` as the **last** constructor parameter (new overload; legacy constructor stays for binary compatibility and delegates with `timeProvider: null`). 3. Initialise the field with `m_timeProvider = timeProvider ??= TimeProvider.System;`. 4. Replace `new Timer(...)`, `Stopwatch`, `HiResClock.*`, `DateTime.UtcNow`-based duration math, and `Task.Delay(delay, ct)` in timing loops with the matching `m_timeProvider` API (`CreateTimer`, `GetTimestamp` / `GetElapsedTime`, `GetUtcNow().UtcDateTime`, `Task.Delay(delay, m_timeProvider, ct)`). 5. Wire the provider through DI: `AddOpcUaServerBuilder` / `AddOpcUaClientBuilder` register `TimeProvider.System` via `TryAddSingleton<TimeProvider>()` so tests can swap in `Microsoft.Extensions.Time.Testing.FakeTimeProvider`. `HiResClock` itself is kept (so the `ApplicationConfiguration.DisableHiResClock` schema flag still round-trips), but **every public member** is now `[Obsolete]` with a single shared message pointing to the new migration-guide section. The few internal call sites that still need to set/read `HiResClock` are wrapped with `#pragma warning disable CS0618`. ### Highlights per component - **Core / Stack** — `Opc.Ua.Core`, `Opc.Ua.Core.Types`, channel pipeline (`UaSCBinaryClientChannel`, `ChannelToken`, `UaSCBinaryChannel`), `ApplicationConfiguration`, certificate validators, `JwksIssuerKeyResolver`, `TimeProviderExtensions` polyfills (`GetTimestampMilliseconds`, `GetTickCount`, `CancellationTokenSource(timeout, timeProvider)`, `Task.Delay(delay, provider, ct)` polyfill for older TFMs). - **Server** — session lockout, subscription pipeline (publish/sample/keep-alive/lifetime), `MonitoredItemQueue`, alarm sampling rate, `IntervalRunner`, server diagnostics, configuration state machines, hosted services. - **Client** — `Session`, managed/V2 subscription engines, reconnect handlers, channel keep-alive, periodic publishing, complex-type discovery polling. - **PubSub** — `IntervalRunner` pipeline, message-receive timers, MQTT/UDP keep-alive, publishers/readers. - **Gds / Lds** — `LdsServer`, `LinqApplicationsDatabase` expiry housekeeping, GDS clients (push, pull), `DirectoryCertificateStore`, `ConnectionStateMachine`, classic subscription engine. - **Quickstarts / Console samples** — `SampleNodeManager`, `ReferenceNodeManager`, `AlarmNodeManager`, `BoilerState`, `MemoryBufferState`, `PublishedValuesWrites`. Samples resolve via `(Server as ITimeProviderProvider)?.TimeProvider ?? TimeProvider.System` so existing ctor signatures stay binary-compatible. - **Tests** — `Opc.Ua.PubSub.Tests` adds `IntervalRunnerTests`; existing `SubscriptionTests`, `UaPublisherTests`, and `TestDataSystem` move off `HiResClock`. Added `Microsoft.Extensions.TimeProvider.Testing` package for `FakeTimeProvider` (project-load warning surfaces on net48/net472 only — runtime is .NET 8+). ### Migration guide New section [**Time and Timer abstraction (`TimeProvider`)**](Docs/MigrationGuide.md#time-and-timer-abstraction-timeprovider) documents: - The canonical replacement table (`HiResClock.UtcNow` → `timeProvider.GetUtcNow().UtcDateTime`, `HiResClock.TickCount64` → `timeProvider.GetTimestamp()`, `new Timer` → `timeProvider.CreateTimer`, `Task.Delay(delay, ct)` → `Task.Delay(delay, timeProvider, ct)`, …). - The `TimeProvider? timeProvider = null` + `??=` constructor pattern. - DI wiring and how to swap in `FakeTimeProvider` in tests. - Before/after snippets for `HiResClock.TickCount64`, `HiResClock.UtcNow`, and `new Timer(...)`. ## Related Issues - Fixes #2458 ## 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. |
||
|
|
bd4543e961 |
ManagedSession, Pluggable Subscription engine, and fluent builder + DI (#3730)
- Add new managed session, wrapping session and providing connection management and failover handling - Add new Subscription engine that can act as alternative to current engine, but is based on channels with worker tasks - Add a fluent builder for managed session clients - Replace NodeCache with LruNodeCache, rename ILruNodeCache and LruNodeCache to INodeCache and NodeCache - Update migration guide and document new functionality. |