Skip to content

Preserve Switch subscription ownership across reentrant handoffs - #1188

Open
dwcullop wants to merge 3 commits into
mainfrom
u/dacullop/main/switch-subscription-ownership
Open

dwcullop wants to merge 3 commits into
mainfrom
u/dacullop/main/switch-subscription-ownership

Conversation

@dwcullop

Copy link
Copy Markdown
Member

Summary

Switch could lose track of which inner subscription owned the current source when the outer observable re-entered during a reset, disposal, or the initial delivery of a newly selected source. A reentrant selection could activate a source that had already been superseded, or dispose the subscription belonging to the newest source, leaving the operator wired to a stale stream or to nothing at all.

Generation-scoped ownership

Each selection of an inner source now carries its own generation identity. Activation, population, and teardown are all validated against the generation that initiated them, so a handoff triggered while an earlier handoff is still unwinding can no longer publish results from, or tear down, a subscription it does not own. A superseded generation quietly retires instead of racing the current one.

Exclusive-resource ordering

The subscription handoff is ordered so that the outgoing subscription is fully released before its replacement starts, rather than overlapping or being released afterwards by whichever path completed last. Outer completion is deferred until the selected inner source terminates, while errors and disposal cancel any activation still pending. The result is a single, unambiguous owner of the inner subscription at every point in the handoff.

Validation

Focused validation for this change was previously run and passed, covering the reentrant reset, disposal, and initial-delivery handoff paths along with the existing Switch suite.

Fixes #1179

@dwcullop

Copy link
Copy Markdown
Member Author

P2: Reentrant initial delivery still permits overlapping exclusive-resource ownership.

Locations: Switch.cs, line 108, and replacement subscription at line 139.

This reproduces on both the PR and its base commit, so it is an unresolved gap, not a regression introduced here. It also limits the newly documented guarantee that the previous subscription is released before its replacement starts.

Reproduction: an inner Observable.Create acquires an exclusive resource and emits synchronously. The downstream OnNext selects a replacement source that needs the same resource. The first Subscribe() has not yet returned its disposable, so previous.Dispose() only marks an empty SingleAssignmentDisposable as disposed. The replacement starts immediately, finds the resource still held, and terminates the switched stream with an error. The first resource is released only when its original subscription call unwinds.

Suggested fix: serialize activation so a replacement selected during an in-progress Subscribe() waits until that call returns and its late disposable has actually been released. Preserve latest-selection semantics while activation is pending, and cancel pending activation on disposal or error. This needs explicit activation-state coordination rather than a different disposable assignment alone. Add a test combining exclusive-resource ownership with reentrant synchronous initial delivery; the current tests exercise those conditions separately.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Reentrant activation can still subscribe the replacement before the outgoing subscription has fully released its resource.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Updates cache Switch subscription ownership to handle reentrant handoffs.

Changes:

  • Adds generation-scoped subscription tracking and teardown ordering.
  • Documents lifecycle behavior.
  • Adds regression tests for reentrant switching and termination.
File Description
ObservableCacheEx.Switch.cs Documents subscription lifecycle semantics.
Cache/​Internal/​Switch.cs Implements generation-scoped handoffs.
SwitchFixture.SubscriptionLifetime.cs Adds lifecycle regression tests.
SwitchFixture.cs Makes the fixture partial.
dynamicdata-cache.instructions.md Updates operator guidance.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

// Subscribe outside the gate and assign only to this generation's holder. If a
// synchronous notification selected a newer source, this holder is already disposed
// and disposes the late-returning subscription without touching the newer one.
subscription.Disposable = source.SubscribeSafe(

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Switch can overlap exclusive subscriptions and discard a reentrantly selected source

2 participants