[romanett]

Description

Fixes an intermittent hang in SubscriptionManager.DisposeAsync() when a Publish worker is blocked on a full ingress buffer. It showed up as ManagerDisposalCancelsSaturatedMessageAdmissionAsync timing out on net48 (about 1 run in 50 locally, and test-windows-net48 (2/19) on the CI run for #5005).

Root cause

The worker's token is linked to the manager's token. Backpressured admission in MessageProcessor.OnPublishReceivedAsync also links the manager's token directly, through its own registration. When the manager is cancelled:

1. The admission callback can run before the callback that cancels the worker's token.
2. The worker resumes inline on the cancelling thread, inside the manager's callback list, and sees ct not yet cancelled.
3. TryBeginPublishRequest() returns false because m_disposed is set, so the loop runs continue without ever awaiting. It spins on the cancelling thread, and the worker's callback can't run.

This spin is normally brief, because the publish controller wakes and disposes the worker, which cancels ct from another thread. On net48 that escape can be lost: CancelAsync is a Task.Run(Cancel) polyfill, so m_publishControl.Set() can wake the controller before the token is flagged. The controller then parks again, and its next wake-up also sits behind the spinning callback, so shutdown never finishes.

On .NET 8+ CancelAsync flags the token synchronously, so from the code the controller should always see it. That explains why this appears on net48, but I did not measure the unfixed rate on net10.0.

Fix

The worker loop now also checks the manager's ShutdownToken, so it exits instead of spinning, whichever order the callbacks run in.

Evidence

A temporary stack trace at the spin showed ctCancelled=False shutdown=True, with PublishWorkerAsync running under CancellationTokenSource.Cancel(). The failing runs had this, and so did many passing runs, where the controller broke the spin.

Build (net48 Release)	Runs	Hangs
Unfixed	110	2
Unfixed, manager cancel delayed 50 ms (temporary injection)	30	13
Fixed, same injection	45	0
Fixed, no injection	60	0

Each row ran as parallel loops on a machine shared with other load. Further runs on the fixed code:

Target	Runs of this test	FullyQualifiedName~Subscription tests
net48 Release	60 passed (table above)	901 passed
net10.0 Release	30 passed	905 passed

There is no evidence that #5000 is involved: nothing it changed appears on the spinning stack. The earlier 0/20 result on the commit before it is consistent with the low failure rate.

Not done

• No new test. The existing test is the regression test, but it only catches this statistically. A deterministic version would need control over callback order and the CancelAsync polyfill's timing.
• The full UA.slnx suite was not run locally, only the tests above, so that checklist item is unchecked.

Related Issues

No issue filed. Found while investigating CI on #5005.

Checklist

• I have signed the CLA and read the CONTRIBUTING doc.
• I have added tests that prove my fix is effective or that my feature works and increased code coverage.
• I have added all necessary documentation.
• I have verified that my changes do not introduce (new) build or analyzer warnings.
• If this PR changes C# source files, I ran all tests locally using the UA.slnx solution against at least .NET Framework and .NET 10, and all passed. PRs changing only non-C# files do not require these .NET test runs.
• 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

[Copilot]

🟢 Approval recommended

No active review issues are recorded for the publish-loop change that lets workers detect manager shutdown directly.

No open issues 🎉

What changed in this PR

Fixes an intermittent SubscriptionManager.DisposeAsync() hang by letting publish workers detect manager shutdown directly, without waiting for cancellation to propagate to their own token.

Changes:

• Check both manager and worker cancellation tokens in the publish loop to prevent spinning on the cancelling thread.
• Document the callback-ordering scenario behind the hang.

File	Description
src/​Opc.Ua.Client/​Subscription/​SubscriptionManager.cs	Adds the manager shutdown check to the publish worker loop and explains why it is needed.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues are reported for the shutdown-cancellation change addressing the intermittent DisposeAsync hang.

No open issues 🎉

_{Review effort: Balanced}

[github-actions]

Code coverage

✅ Coverage gate passed.

Check	Result	Threshold
✅ Project line rate	88.90% (464020/521985 lines)	>= 70.00%
✅ Project branch rate	79.30%	>= 60.00%
✅ Patch coverage	100.00% (2/2 changed lines)	>= 50.00% (<= 10 changed lines, advisory)
ℹ️ Baseline delta (advisory)	+15.30 pp	73.60% recorded

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

_{Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 88.20%, branch 78.61%.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.28%. Comparing base (74728d8) to head (0e3c1db).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5007      +/-   ##
==========================================
- Coverage   83.29%   83.28%   -0.02%     
==========================================
  Files        3589     3607      +18     
  Lines      520073   521985    +1912     
  Branches    88220    88650     +430     
==========================================
+ Hits       433182   434718    +1536     
- Misses      57533    57736     +203     
- Partials    29358    29531     +173     

Flag	Coverage Δ
actions	83.28% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
.../Opc.Ua.Client/Subscription/SubscriptionManager.cs	83.64% <100.00%> (+0.13%)	⬆️

... and 72 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.