Skip to content

Release property handlers when initialization throws - #1193

Open
dwcullop wants to merge 1 commit into
mainfrom
u/dacullop/main/release-property-handlers-on-init-throw
Open

dwcullop wants to merge 1 commit into
mainfrom
u/dacullop/main/release-property-handlers-on-init-throw

Conversation

@dwcullop

Copy link
Copy Markdown
Member

Problem

WhenPropertyChanged / WhenValueChanged attached PropertyChanged handlers and emitted the initial value from the subscription's constructor. If anything threw during that work (a property getter, or a subscriber callback receiving the initial value), the exception escaped before Observable.Create could hand back a disposable. Rx had nothing to dispose, so every handler installed on the way in stayed attached and kept the observed objects alive.

Fix: ownership before activation

The subscription object is now constructed first, scoped by a RefCountDisposable, and only then started:

var subscription = new SinglePropertySubscription(observer, source, memberName, accessor);
using var lifetime = new RefCountDisposable(subscription);
subscription.Start(notifyInitial);
return lifetime.GetDisposable();

The scope owns initialization. If Start throws, the using releases the subscription and detaches every handler attached so far, at every level of a deep chain. Only successful activation hands a dependent lease to Rx. The original exception still propagates unchanged, and subscriber callback failures are never converted into OnError.

Both the shallow single-property path and the deep-chain path are covered; DeepChainSubscription gains the same Start() seam so its chain walk runs under the same ownership scope.

Tests

WhenPropertyChangedBehaviorFixture.Initialization.cs adds regression coverage for a throwing initial observer, a throwing getter under both the default and a handled error path, shallow and deep chains, explicit disposal, and synchronous completion via Take(1). Each asserts handler counts drop to zero while confirming handlers attach before the value is read. Prior focused validation of these tests passed.

Fixes #1176

Property observation attached PropertyChanged handlers and emitted the initial value from the subscription constructor, so a failure during initialization escaped before Observable.Create could return a disposable. Every handler installed on the way in stayed attached, leaking the observed objects.

Ownership is now established before activation: the subscription is constructed, scoped by a RefCountDisposable, and only started afterwards. Rx receives a dependent lease only once activation succeeds, so a throwing getter or subscriber callback releases every handler at every chain level while the original exception still propagates unchanged.
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]: Property observation leaks event handlers when initial subscription throws

1 participant