Why do I need to dispose of subscriptions after completion?

Question

The Intro To RX book describes the return value on OnSubscribe as IDisposible and notes that subscriptions should be disposed of when OnError and OnCompleted are called.

An interesting thing to consider is that when a sequence completes or errors, you should still dispose of your subscription.

From Intro to RX: Lifetime Management, OnError and OnCompleted

Why is this?

---

For reference, this is the class I'm currently working on. I'm probably going to submit it to code review at some point.

using System;
using System.Reactive;
using System.Reactive.Linq;
using System.Reactive.Subjects;

/// <summary>
/// Provides a timeout mechanism that will not timeout if it is signalled often enough
/// </summary>
internal class TrafficTimeout
{
    private readonly Action onTimeout;
    private object signalLock = new object();
    private IObserver<Unit> signals;

    /// <summary>
    /// Initialises a new instance of the <see cref="TrafficTimeout"/> class.
    /// </summary>
    /// <param name="timeout">The duration to wait after receiving signals before timing out.</param>
    /// <param name="onTimeout">The <see cref="Action"/> to perform when the the timeout duration expires.</param>
    public TrafficTimeout(TimeSpan timeout, Action onTimeout)
    {
        // Subscribe to a throttled observable to trigger the expirey
        var messageQueue = new BehaviorSubject<Unit>(Unit.Default);
        IDisposable subscription = null;
        subscription = messageQueue.Throttle(timeout).Subscribe(
        p =>
        {
            messageQueue.OnCompleted();
            messageQueue.Dispose();
        });

        this.signals = messageQueue.AsObserver();
        this.onTimeout = onTimeout;
    }

    /// <summary>
    /// Signals that traffic has been received.
    /// </summary>
    public void Signal()
    {
        lock (this.signalLock)
        {
            this.signals.OnNext(Unit.Default);
        }
    }
}

Answer 1

The disposable returned by the Subscribe extension methods is returned solely to allow you to manually unsubscribe from the observable before the observable naturally ends.

If the observable completes - with either OnCompleted or OnError - then the subscription is already disposed for you.

Try this code:

var xs = Observable.Create<int>(o =>
{
    var d = Observable.Return(1).Subscribe(o);
    return Disposable.Create(() =>
    {
        Console.WriteLine("Disposed!");
        d.Dispose();
    });
});

var subscription = xs.Subscribe(x => Console.WriteLine(x));

If you run the above you'll see that "Disposed!" is written to the console when the observable completes without you needing call .Dispose() on the subscription.

One important thing to note: the garbage collector never calls .Dispose() on observable subscriptions, so you must dispose of your subscriptions if they have not (or may not have) naturally ended before your subscription goes out of scope.

Take this, for example:

var wc = new WebClient();

var ds = Observable
    .FromEventPattern<
        DownloadStringCompletedEventHandler,
        DownloadStringCompletedEventArgs>(
            h => wc.DownloadStringCompleted += h,
            h => wc.DownloadStringCompleted -= h);

var subscription =
    ds.Subscribe(d =>
        Console.WriteLine(d.EventArgs.Result));

The ds observable will only attach to the event handler when it has a subscription and will only detach when the observable completes or the subscription is disposed of. Since it is an event handler the observable will never complete because it is waiting for more events, and hence disposing is the only way to detach from the event (for the above example).

When you have a FromEventPattern observable that you know will only ever return one value then it is wise to add the .Take(1) extension method before subscribing to allow the event handler to automatically detach and then you don't need to manually dispose of the subscription.

Like so:

var ds = Observable
    .FromEventPattern<
        DownloadStringCompletedEventHandler,
        DownloadStringCompletedEventArgs>(
            h => wc.DownloadStringCompleted += h,
            h => wc.DownloadStringCompleted -= h)
    .Take(1);

I hope this helps.

Answer 2

Firstly, here's an example of the trouble you can run into:

void Main()
{
    Console.WriteLine(GC.GetTotalMemory(true));
    for (int i = 0; i < 1000; i++)
    {
        DumbSubscription();
        Console.WriteLine(GC.GetTotalMemory(true));
    }
    Console.WriteLine(GC.GetTotalMemory(true));
}

public void DumbSubscription()
{
    Observable.Interval(TimeSpan.FromMilliseconds(50))
        .Subscribe(i => {});
}

You will see your memory usage go up forever. Active Rx subscriptions do not get garbage collected and this observable is infinite. Therefore, if you increase the loop limit, or add a delay, and you'll simply have more wasted memory: Nothing will help you except for disposing of those subscriptions.

However, let's say we change the definition of DumbSubscription to this:

public void DumbSubscription()
{
    Observable.Interval(TimeSpan.FromMilliseconds(50))
        .Take(1)
        .Subscribe(i => {});
}

The .Take(1) addition means that the observable will complete after one interval, so it's no longer infinite. You'll see that your memory usage stabilizes: Subscriptions tend to properly dispose of themselves upon completion or exception.

However, that doesn't change the fact that, like any other IDisposable, it's a best practice to call Dispose (either manually or via using) to make sure resources are properly disposed of. Additionally, if you tweak your observable, you can easily run into the memory leak problem pointed out at the beginning.