Does ReactiveUI leak subscriptions?
Asked Answered
C

3

10

I've looked at the examples of ReactiveUi from the blogs and I am left wondering if ReactiveUI has some sort of subscription management facility underneath or are the examples ignoring the fact that they might leak subscriptions?

Any time I call a method in ReactiveUi that results in an IDisposable, do I need to hold onto that reference and track it myself? Also does this mean my ViewModels need to be disposable, this seems difficult since we don't really know when the connected "Views" go away (i.e. if my ViewModel reflects items in a data grid) in WPF so there seems to be no appropriate place to call dispose.

Chicken answered 18/1, 2012 at 20:45 Comment(0)
C
12

You only need to hold references to the IDisposable returned by subscriptions if you need to unsubscribe early from an observable. Observables will naturally call Dispose when they terminate by either the OnCompleted or OnError messages.

You do, however, need to keep references when you have an infinite observable subscription (i.e. FromEventPattern), but this is exactly the same as needing to remove event handlers before you close a form/view.

Condensation answered 19/1, 2012 at 0:41 Comment(2)
Typically you don't have to remove event handlers upon form closing if publisher's lifetime is same or smaller than lifetime of subscriber. Does same principle apply to ReactiveCommand for example?Fastigium
@Fastigium - I would suggest that you do explicitly dispose of all Rx subscriptions if you know that some might still be running just to be safe. It's easy enough to have one form-level CompositeDisposable that you track all the subscriptions. It means just a single .Dispose() when you exit. Like regular events, you might get away without doing so but it's not always the case.Condensation
E
15

You also have to remember, that the IDisposables returned by Rx and ReactiveUI aren't associated with unmanaged memory - it's all just simple .NET objects, still ref'ed by the garbage collector.

Most of the Subscriptions you make in the constructors of your ReactiveObjects will be tied to the lifetime of the host object - so when it goes out of scope and is subject to GC, so will all the subscriptions, the CLR will detect the circular reference and just nuke everything.

As Enigmativity mentions, the one tricky bit is when you use FromEventPattern to tie the lifetime of a Subscription (and perhaps, a ViewModel) to the lifetime of a WPF object. However, I'd argue if you're using FromEventPattern often in ReactiveUI, you are most definitely Doing It Wrong™.

RxUI is all about ViewModels, and ViewModels are all about commands and properties (and wiring up how properties are related to each other), so you can test the behavior of user experience separately from its visuals.

Extraneous answered 26/1, 2012 at 7:32 Comment(5)
But isn't the usage of Properties and the ObservedChanged "events" just using from EventPattern on the PropertyChanged and PropertyChanging events? And isn't the usage of Commands hooking into the Executed event? I guess this is what worries me.Chicken
Nope, internally ReactiveObject uses a Subject, you're not using FromEventPattern when you use ObservableForProperty or WhenAnyExtraneous
Okay. Thanks for the clarification. I'm marking Enigmativity's answer as the answer since he was right first, but your clarification was crucial to my convert level.Chicken
@PaulBetts Unfortunately you are wrong about the FromEventPattern and INPC events. I think if you follow the code back from WhenAny it will be a bunch of RX magic leading back to a FromEventPattern on the PropertyChangedEvent. This code all changed when I did a major rewrite of WhenAny.Graphemics
I've got a situation in my code where I have a mouse move event (FromEventPattern) bound via some transformation to a property on a ViewModel in a reactive collection which is rendered out via an ItemsControl. It leaks bad and view models that are removed from the collection still get events from the binding ( which was defined in XAML ) I think this is a XAML binding bug/feature as well.Graphemics
C
12

You only need to hold references to the IDisposable returned by subscriptions if you need to unsubscribe early from an observable. Observables will naturally call Dispose when they terminate by either the OnCompleted or OnError messages.

You do, however, need to keep references when you have an infinite observable subscription (i.e. FromEventPattern), but this is exactly the same as needing to remove event handlers before you close a form/view.

Condensation answered 19/1, 2012 at 0:41 Comment(2)
Typically you don't have to remove event handlers upon form closing if publisher's lifetime is same or smaller than lifetime of subscriber. Does same principle apply to ReactiveCommand for example?Fastigium
@Fastigium - I would suggest that you do explicitly dispose of all Rx subscriptions if you know that some might still be running just to be safe. It's easy enough to have one form-level CompositeDisposable that you track all the subscriptions. It means just a single .Dispose() when you exit. Like regular events, you might get away without doing so but it's not always the case.Condensation
G
2

Just to be safe I will be using this pattern from now on. I will probably modify to work via extension methods but the principle is sound. To be sure you don't leak subscriptions you want to drop them

class Mesh2D{

    public Mesh2D()
    {
        DisposeOnUnload(CreateBindings());
    }

    // Register all disposables for disposal on
    // UIElement.Unload event. This should be
    // moved to an extension method.
    void DisposeOnUnload(IEnumerable<IDisposable> disposables)
    {
        var d = new CompositeDisposable(disposables);
        var d2 = this.UnloadedObserver()
            .Subscribe(e => d.Dispose());
        var d3 = this.Dispatcher.ShutdownStartedObserver()
            .Subscribe(e => d.Dispose());
        d.Add(d2);
        d.Add(d3);
    }

    // Where your bindings are simply yielded and
    // they are removed on Unload magically
    IEnumerable<IDisposable> CreateBindings()
    {
        // When the size changes we need to update all the transforms on 
        // the markers to reposition them. 
        yield return this.SizeChangedObserver()
            .Throttle(TimeSpan.FromMilliseconds(20), RxApp.DeferredScheduler)
            .Subscribe(eventArgs => this.ResetImageSource());

        // If the points change or the viewport changes
        yield return this.WhenAny(t => t.Mesh, t => t.Viewport, (x, t) => x.Value)
            .Throttle(TimeSpan.FromMilliseconds(20), RxApp.DeferredScheduler)
            .Subscribe(t => this.UpdateMeshChanged());
    }
}

Note I'm wrapping RX FromEventPattern with automatically generated extension methods so I get SizeChangedObserver() and UnloadedObserver() for free rather than the hard to remember format of FromEventPattern.

The wrapping code is generated as such

public static IObservable<EventPattern<RoutedEventArgs>> UnloadedObserver(this FrameworkElement This){
    return Observable.FromEventPattern<RoutedEventHandler, RoutedEventArgs>(h => This.Unloaded += h, h => This.Unloaded -= h);
}

The above pattern could probably be used to unbind IDisposable view models as well.

Graphemics answered 26/3, 2013 at 16:43 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.