24

I am investigating the use of Observable.Generate to create a sequence of results sampled at intervals using the examples from the msdn website as a starting point.

The following code WITHOUT a TimeSpan selector does not exhibit a memory leak:

IObservable<string> obs = Observable.Generate(initialState: 1,
                                              condition: x => x < 1000,
                                              iterate: x => x + 1,
                                              resultSelector: x => x.ToString());
obs.Subscribe(x => Console.WriteLine(x));

However, the following code WITH a TimeSpan selector exhibits a memory leak:

TimeSpan timeSpan = TimeSpan.FromSeconds(1);
IObservable<string> obs = Observable.Generate(initialState: 1,
                                              condition: x => x < 1000,
                                              iterate: x => x + 1,
                                              resultSelector: x => x.ToString(),
                                              timeSelector: x => timeSpan);
obs.Subscribe(x => Console.WriteLine(x));

For example, this toy app will quickly show the memory leak using the Memory Profiler which ships with VS 2015 Community:

using System;
using System.Reactive.Linq;

namespace Sample
{
    public class Program
    {
        static void Main()
        {
            IObservable<string> obs = Observable.Generate(1, x => x < 1000*1000, x => x + 1, x => x.ToString(), x => TimeSpan.FromMilliseconds(500));
            obs.Subscribe(x => { /*Do nothing but simply run the observable*/ });
            Console.ReadLine();
        }
    }
}

The memory leak is a growing collection of:

System.Reactive.Disposables StableCompositeDisposable.Binary
System.Reactive.Disposables SingleAssignmentDisposable

Am I using this API incorrectly? Should I expect the memory to grow or is this a bug with Reactive?

James World
  • 29,019
  • 9
  • 86
  • 120
  • Probably multiple subscriptions are being made. Is the `Observable` by any chance constructed within an event handler? – supertopi Dec 19 '16 at 13:41
  • 6
    The amount of up-votes is puzzling because anyone can copy-paste and run this code and see that there is no memory leak... :) – supertopi Dec 19 '16 at 13:58
  • 1
    @supertopi depends on where the `timeSpan` is declared. – Jeroen van Langen Dec 19 '16 at 14:02
  • >Am I using this API incorrectly? Well how are you using the API? A comment here suggests that the memory leak depends on where you declare the TimeSpan. In that case you need to elaborate so that others can actually reproduce your issue. http://stackoverflow.com/help/mcve – Lee Campbell Dec 19 '16 at 14:45
  • I have updated the question with a simple program example. Appreciate any thoughts and advice! Many thanks all. – Benjamin Osborne Dec 19 '16 at 16:00
  • I've tried with the updated sample. No repro. – Asti Dec 19 '16 at 17:35
  • Note: I've reopened this, as it is not a duplicate of http://stackoverflow.com/questions/32641775/why-does-this-observable-generate-overload-cause-a-memory-leak?noredirect=1&lq=1 - the subtle difference in the TimeSpan means the example and answer are different. – James World Dec 20 '16 at 14:19

1 Answers1

6

This does look like a bug to me - or at least messy/undesirable behaviour in the DefaultScheduler's "recursive" scheduling implementation (it's not really recursive, I'm talking about the overload that passes in the scheduler itself to a scheduled action so you can schedule a continuation).

The disposables you are seeing build up are created by the call to the DefaultScheduler.Schedule method (line 71 here: https://github.com/Reactive-Extensions/Rx.NET/blob/master/Rx.NET/Source/System.Reactive.Core/Reactive/Concurrency/DefaultScheduler.cs).

There are a couple of reasons why other attempts here to spot this failed. Firstly, the disposables ARE eventually disposed - but only when the Generate OnCompletes or OnErrors, at which point the System.Reactive.AnonymousSafeObserver<T> returned by Generate when you subscribe to it does it's clean up.

Secondly, if you use a short TimeSpan (remember the .NET Timer minimum resolution is 15ms anyway) then Rx will optimize away the use of a timer and call QueueUserWorkItem with no timer being used so these disposables don't ever get created.

If you dig into Generate's implementation (https://github.com/Reactive-Extensions/Rx.NET/blob/master/Rx.NET/Source/System.Reactive.Linq/Reactive/Linq/Observable/Generate.cs) you can see that it passes the IDisposable returned by the initial call to Schedule passing it back to the observer which hangs on to it until error/completion. That prevents the entire resulting chain of recursive calls being collectable - and means that if you do need to cancel, or when clean-up happens, only then will every scheduled action's disposable be disposed.

You can see the same effect in the code below which uses the DefaultScheduler directly - the reference to cancel in the last line is enough to cause the leak. Make sure to use a release build otherwise the compiler will keep hold of cancel until the method end regardless.

// ensure you are using a release build of this code
ManualResetEvent mre = new ManualResetEvent();
IDisposable cancel;
int maxCount = 20;

TimeSpan timeSpan = TimeSpan.FromSeconds(1);

Func<IScheduler, int, IDisposable> recurse = null;
recurse = (self, state) =>
{
    Console.WriteLine(state);

    if (state == maxCount)
    {
        mre.Set();
        return Disposable.Empty;
    }

    return self.Schedule(state + 1, timeSpan, recurse);
};

cancel = Scheduler.Default.Schedule(1, timeSpan, recurse);

mre.WaitOne();

// uncomment the following line, and you'll get the same leak
// leave it commented, and cancel reference is GC'd early and there's no leak
// if(cancel == null) Console.WriteLine("Hang on to cancel");

I used Jetbrains dotMemory API to take memory dumps to draw conclusions here - I've stripped the code above of those API calls, but there is a full gist here if you have that product, and you'll be able to see the impact of uncommenting the final line quite clearly: https://gist.github.com/james-world/f20377ea610fb8fc0ee811d27f7a837c Alternatively, you could use the MS profiler API - which I don't have paged into my brain's working set at the moment!

James World
  • 29,019
  • 9
  • 86
  • 120
  • 2
    "which I don't have paged into my brain's working set" I will be using this phrase from now on. And as the years go by, probably more often! – Sentinel Apr 17 '18 at 07:56
  • Actually could you please help me out with this one https://stackoverflow.com/q/49880719/442396 ? I am somehow stuck, though it ought to be simple. – Sentinel Apr 17 '18 at 14:30