mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-18 14:43:08 +08:00
romanett
2ed505547b
## Proposed changes `ManagedSessionReconnectTests.CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAliveAsync(False)` failed on #4594 (a workflow-YAML-only PR) in [run 36590762260](https://github.com/OPCFoundation/UA-.NETStandard/actions/runs/36590762260), `test-windows-net10.0 (5/30)`: ``` Assert.That(harness.Logs.Records.Any( record => record.EventId.Name == "ManagedSessionKeepAliveFailureSuppressedWhile"), Is.False) Expected: False But was: True ``` ### Root cause (test race, not a product bug) The suppressed record doesn't come from the failed keep-alive the test checks. It comes from the recovery that this failure starts: 1. The scripted keep-alive read returns `BadNoCommunication`. `ManagedSession.OnInnerKeepAlive` does not suppress it and calls `TriggerReconnect`, as expected. 2. The outer recovery reactivates through the channel manager. When `Session.ReconnectCoreAsync` finishes, `StartKeepAliveTimerAsync` restarts the keep-alive timer with due time 0, which sends a read right away. 3. The test reset `KeepAliveStatus` to Good only in the continuation after `FailedKeepAlive`. That continuation runs asynchronously, so the new read can still return Bad. 4. `ChannelEntry` goes `TransitionTo(Ready)` → `SignalReady()` → `CompleteParticipantRecoveryAsync`, and only releases `m_reconnectCoalescer` afterwards. A read released by `SignalReady` that fails inside that window sees `ChannelRecoveryInProgress == true`. Suppressing it there is intended: the manager is still restoring behind its provisional Ready gate. The assertion scanned every log record, so it also caught this correctly suppressed failure from the follow-up recovery. **Proof of causality:** adding `Task.Delay(100)` after `SignalReady()` in `ChannelEntry`, plus `Task.Delay(50)` before the test reset `KeepAliveStatus`, made both test cases fail every time with the CI assertion. The log dump showed `KeepAliveReadFailed` → `ManagedSessionKeepAliveFailureSuppressedWhile` right after the second recovery's `Reactivating → Ready`. Both delays have been removed. ### Fix - Check the suppression decision synchronously in a `KeepAlive` handler. `ManagedSession` raises that event right after it decides whether to suppress. The assertion now covers only the failure under test. - Reset `KeepAliveStatus` to Good in that same handler, before the triggered recovery can send another read. ## Types of changes - [x] Bugfix (non-breaking change which fixes an issue) ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf). - [x] I ran tests locally with my changes, all passed. Local verification (net10.0): - With the product-side repro delay still in place, both cases pass with the fix. - `ManagedSessionReconnectTests` (17 tests): 30 sequential runs, all green. - `CancelledRecoveryCanBeReplacedWithoutSuppressingLaterKeepAlive*`: 64 runs across 8 parallel workers, all green. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>