[Server] Make subscription transfer between sessions transactional

[marcschier]

This is PR 7 of the stacked series splitting integration PR #4093, stacked on #4147 (marcschier/wot-05-lifecycle).

This PR makes subscription transfer between sessions transactional: subscriptions are prepared before any ownership move, publish-queue transfer claims block stale source-session publishing, and rollback restores monitored-item resend-data trigger state.

ServerInternalData also gains asynchronous disposal so the asynchronously disposable subscription manager is shut down without blocking, with sync and async dispose paths guarded so repeated disposal is a no-op.

It also brings the transfer-focused coverage for subscription transfer, publish-queue transfer claims, and ServerInternalData async/double-disposal behavior.

The immediate monitored-item retirement mechanism present in #4093 is deliberately excluded per maintainer decision. IRetirableMonitoredItem and INodeManagerMonitoredItemRetirementTracker do not appear in this PR.

[Copilot]

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Makes subscription transfer between sessions transactional and adds async disposal support to avoid blocking shutdown, with accompanying test coverage.

Changes:

• Add transfer-claim + prepared-transfer flow to prevent stale source publishing and enable rollback of monitored-item resend state.
• Introduce IAsyncDisposable for SubscriptionManager and ServerInternalData, with idempotent dispose paths and worker shutdown coordination.
• Add/enable tests covering publish-queue concurrency, transfer claims, and async/double-dispose behavior.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/Opc.Ua.Subscriptions.Tests/SessionPublishQueueConcurrencyTests.cs	New concurrency tests around publish/requeue/expiration and transfer-claim behavior.
tests/Opc.Ua.Server.Tests/SubscriptionTests.cs	Adds async-dispose tests and multiple transfer transaction/rollback scenarios.
tests/Opc.Ua.Server.Tests/ServerInternalDataTests.cs	Adds async-dispose and double-dispose coverage for ServerInternalData.
tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs	Re-enables rollback test previously ignored.
src/Opc.Ua.Server/Subscription/SubscriptionManager.cs	Implements async disposal, worker cancellation/joining, and transactional transfer orchestration.
src/Opc.Ua.Server/Subscription/Subscription.cs	Adds transfer reservation and prepared transfer (commit/rollback) plus transfer-in-progress guards.
src/Opc.Ua.Server/Subscription/SessionPublishQueue.cs	Adds transfer-claim mechanism to block stale publishing and coordinate expiration/requeue.
src/Opc.Ua.Server/Subscription/MonitoredItem/MonitoredItem.cs	Implements resend-trigger restoration interface for transfer rollback.
src/Opc.Ua.Server/Subscription/MonitoredItem/IMonitoredItem.cs	Adds internal IMonitoredItemTransferState contract for rollback.
src/Opc.Ua.Server/Server/ServerInternalData.cs	Adds async disposal path that attempts to dispose subscription manager asynchronously.
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs	Captures/restores resend trigger state during monitored-item transfer rollback.
src/Opc.Ua.Server/Hosting/OpcUaServerHostedService.cs	Minor typing/namespace adjustments.

[codecov]

Codecov Report

❌ Patch coverage is 68.69565% with 180 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.89%. Comparing base (d3c8030) to head (d676000).

Files with missing lines	Patch %	Lines
.../Opc.Ua.Server/Subscription/SubscriptionManager.cs	65.94%	54 Missing and 25 partials ⚠️
src/Opc.Ua.Server/Subscription/Subscription.cs	62.22%	54 Missing and 14 partials ⚠️
.../Opc.Ua.Server/Subscription/SessionPublishQueue.cs	72.27%	21 Missing and 7 partials ⚠️
src/Opc.Ua.Server/Server/ServerInternalData.cs	93.75%	0 Missing and 3 partials ⚠️
.../Opc.Ua.Server/Hosting/OpcUaServerHostedService.cs	0.00%	1 Missing ⚠️
...Server/Subscription/MonitoredItem/MonitoredItem.cs	83.33%	0 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (68.69%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@                       Coverage Diff                       @@
##           marcschier/wot-05-lifecycle    #4150      +/-   ##
===============================================================
- Coverage                        79.91%   79.89%   -0.03%     
===============================================================
  Files                             1527     1527              
  Lines                           212839   213287     +448     
  Branches                         36693    36766      +73     
===============================================================
+ Hits                            170090   170405     +315     
- Misses                           29983    30078      +95     
- Partials                         12766    12804      +38     

Files with missing lines	Coverage Δ
src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs	84.12% <100.00%> (+1.20%)	⬆️
.../Opc.Ua.Server/Hosting/OpcUaServerHostedService.cs	76.92% <0.00%> (ø)
...Server/Subscription/MonitoredItem/MonitoredItem.cs	84.02% <83.33%> (+0.08%)	⬆️
src/Opc.Ua.Server/Server/ServerInternalData.cs	91.56% <93.75%> (+0.75%)	⬆️
.../Opc.Ua.Server/Subscription/SessionPublishQueue.cs	81.05% <72.27%> (-4.62%)	⬇️
src/Opc.Ua.Server/Subscription/Subscription.cs	87.95% <62.22%> (-4.47%)	⬇️
.../Opc.Ua.Server/Subscription/SubscriptionManager.cs	79.73% <65.94%> (-3.86%)	⬇️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:

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

[marcschier]

Patch coverage: 76.96 % → 89.08 % (gate is 80 %)

Measured locally by merging the Cobertura reports from Opc.Ua.Server.Tests and
Opc.Ua.Subscriptions.Tests (net10.0) against origin/marcschier/wot-05-lifecycle:

	before	after
patch coverage	76.96 % (451 / 586)	89.08 % (522 / 586)
Subscription/SubscriptionManager.cs	174 / 232 (75.0 %)	192 / 232 (82.8 %)
Subscription/Subscription.cs	133 / 187 (71.1 %)	168 / 187 (89.8 %)
Subscription/SessionPublishQueue.cs	82 / 104 (78.8 %)	100 / 104 (96.2 %)

71 previously uncovered changed lines are now executed. All of them are transfer
failure paths — the behaviour this PR exists to guarantee.

What the new tests cover

13 tests were added to tests/Opc.Ua.Server.Tests/SubscriptionTests.cs. No production
code was touched.

Publish-queue transfer claims (SessionPublishQueue.cs, 18 lines: 284, 313, 341,
344-348, 507-509, 701-709)

• TryClaimForTransferFailsWhenTheClaimingSessionIsNotTheOwner — a claim made on behalf
  of a session that no longer owns the subscription is refused, the entry stays
  publishable, and the refusal does not block the real owner from transferring later.
• RestoreTransferClaimFailsWhenTheClaimWasAlreadyRestored — a claim can only be
  restored once, so a stale rollback cannot overwrite a newer queue entry.
• TryRemoveForTransferRemovesOnlyTheExactQueuedEntry — removal is identity-based: a
  different ISubscription instance carrying the same id must not evict the entry, and
  a subscription already gone cannot be removed twice.
• PublishCompletedKeepsNotificationsOfAClaimedSubscriptionAsync — a notification that
  arrives while the entry is held under a transfer claim is recorded on the claim and
  is still published by the source session once the transfer is abandoned. This is the
  "cannot be lost if the transfer is abandoned" guarantee.

Subscription-level transfer state machine (Subscription.cs, 35 lines: 639,
805-811, 896, 927-929, 1032-1034, 1097-1099, 1126-1179)

• TryBeginTransferFailsWhileATransferIsAlreadyInProgress and
  PublishTimerIsIdleWhileATransferIsInProgress — one reservation at a time, and a
  reserved subscription does not expire on the source session's publish timer.
• PrepareSessionTransferAsyncRejectsAnUnreservedSubscription and
  CompleteTransferRejectsAnUnexpectedOwner — both raise BadSubscriptionIdInvalid
  when the reservation or the owner is not what the caller assumed.
• TransferSessionAsyncMovesOwnershipToTheDestinationAsync — covers the legacy
  non-transactional path that had no direct test.
• FailedOwnershipCommitRollsBackThePreparedTransferAsync — the reservation is dropped
  while monitored items are being handed over, so CommitOwnership fails after
  preparation succeeded. This drives PreparedSessionTransfer.RollbackAsync end to end
  (ownership restore, diagnostics restore, monitored-item resend-state rollback, source
  context disposal), which previously had zero coverage. The test asserts the source
  session is still the owner, the subscription is back in the source publish queue, and
  a retry then transfers cleanly.

Manager-level rollback and claim handling (SubscriptionManager.cs, 18 lines:
1702-1708, 1734-1740, 1841-1842, 1864-1867, 1893-1894)

• TransferFailsWhenTheSourceQueueEntryIsAlreadyClaimedAsync — a TryClaimForTransfer
  that loses the race returns BadSubscriptionIdInvalid for that subscription and
  leaves ownership untouched; the retry after the claim is released succeeds.
• TransferFailsWhenTheAbandonedSubscriptionIsAlreadyReservedAsync — same for an
  abandoned (closed-session) subscription: it stays in m_abandonedSubscriptions
  rather than being dropped on the floor.
• FailedSourceQueueRestoreAggregatesTheTransferErrorAsync — the source queue slot is
  occupied while the transfer is in flight, so RestoreTransferClaim cannot put the
  entry back. The manager reports the rollback failure aggregated with the original
  transfer error instead of silently losing the subscription, and no stale claim is
  left behind to block the next transfer.

All tests are deterministic — no Thread.Sleep, no wall-clock or parallel-timing
dependence; the failure injection is done through the mocked IMasterNodeManager
callback and through the internal claim APIs.

Test runs

project	TFM	result
Opc.Ua.Server.Tests	net10.0	4008 passed, 8 skipped, 1 failed
Opc.Ua.Server.Tests	net48	4008 passed, 8 skipped, 1 failed
Opc.Ua.Subscriptions.Tests	net10.0	604 passed, 6 skipped, 0 failed

The two failures are the known ones and are unrelated to this commit:
LiveNodeManagerAddRefreshesTheClientNamespaceTable (regression inherited from base
PR #4147, reproduces there too) on net10.0, and
DurableDataValueQueueVerifyReferenceBatchingAsync on net48.

dotnet format (whitespace, style, analyzers) reports no changes for the edited file,
and both projects build with 0 warnings under TreatWarningsAsErrors.

[romanett]

A transfer is a single method call for a client, and a nodemanager has no reason to not complete a transfer, why should the sdk implement logic for wrongly implemented node managers?
Or did i miss a benefit this brings?

Clients calling transfer multiple times?

[marcschier]

@romanett fair question — the machinery is not there to defend against wrongly implemented NodeManagers. Three concrete reasons, all of which are reachable with perfectly correct NodeManagers.

1. A transfer is not one call — it fans out across every owning NodeManager.

A single subscription's monitored items can be spread over several NodeManagers, so MasterNodeManager groups the items by owner and calls each one in turn (MasterNodeManager.cs:6144 builds the owner groups; :6178 and :6238 iterate them). If owner 3 of 5 returns an error, owners 1 and 2 have already moved their items. Without prepare/commit/rollback the subscription is left permanently half-owned — some items answering to the destination session, the rest still to the source. That is a correctness hole in the SDK, not in the NodeManager.

2. There are legitimate reasons for a NodeManager to refuse.

It is not "no reason to not complete". A node can be deleted between the client's browse and its TransferSubscriptions, giving BadNodeIdUnknown. The destination session's identity may not have access to a node the source could read. And in this very stack a NodeManager can be reloaded or replaced at runtime (AddRuntimeNodeSetAsync, shadow reload, immediate reload), so items can legitimately become unresolvable mid-transfer. Those are correct refusals that still have to leave consistent state.

3. Yes — repeated and racing transfers are real.

Directly answering "clients calling transfer multiple times": a client that times out and retries produces exactly that, and two sessions can race to transfer the same subscription. The transfer claim is what makes those safe:

• it stops the source session publishing stale notifications while the move is in flight;
• it stops the subscription expiring on the source publish timer while it is reserved;
• notifications that arrive mid-transfer are held on the claim and are still delivered by the source if the transfer is abandoned — otherwise they are silently lost.

The tests added for the coverage gate exercise precisely these paths, all through the public/mocked seams rather than by simulating a broken NodeManager: TransferFailsWhenTheSourceQueueEntryIsAlreadyClaimedAsync (lost claim race), TransferFailsWhenTheAbandonedSubscriptionIsAlreadyReservedAsync, PublishCompletedKeepsNotificationsOfAClaimedSubscriptionAsync (no notification loss on abandon), and FailedOwnershipCommitRollsBackThePreparedTransferAsync (partial failure across owners restores ownership, diagnostics and monitored-item resend state, and a retry then succeeds).

So the benefit is: a partially-failed transfer leaves the subscription exactly where it started rather than split across two sessions, and a retry works. Happy to trim anything that is not pulling its weight — but I would not drop the prepare/rollback across multiple owners, because that failure mode does not require anyone to have implemented a NodeManager badly.

Replied to concern