OperatorSwitch keeping reference to previous Observable longer than expected

[Thomvis]

Hi & thanks for this amazing library!

For the past few days I've been confused by some memory leaks that caused by RxJava or, more likely, my misunderstanding of RxJava. I've reduced the issue to a very simple test case that you can find here: https://gist.github.com/Thomvis/18474b51b1b4dff79191f93330ae627e.

The issue has to do with switchMap. I'd expect that when the source Observable emits an onNext event (containing a new Observable), the operator will unsubscribe from the previous Observable and release all references to that Observable. The unsubscribing part is happening, but the releasing is not. Because the Observable is not released, any upstream operators, anonymous inner classes and their captures are also not released and subsequently not garbage collected.

I've read #1292 and #3074, but I'm not sure if they're related.

So what I'm hoping to get from this issue is one of the following:

• My test is wrong
• I'm using switchMap wrong / have wrong assumptions on the memory model
• (There's a bug in OperatorSwitch)

Thanks in advance,
Thomas

[davidmoten]

I had a quick look at OperatorSwitch and didn't pick up on any obvious retention of references. One thing I notice from your test is that you can't assume System.gc() is run synchronously and to have a hope of it doing something I imagine you'd need to put a sleep in after it.

[Thomvis]

Thanks for the super quick response!

I re-ran the tests with a 1 and 10 second sleep after each System.gc(), but the results were the same.

The internals of RxJava are still a bit over my head, but looking at the following path from a heap dump, it could have something to do with the arbiter keeping a reference to the producer. Note that this dump is from the real app I'm building, not the reduced test case.

In the context of the heap dump from the image, I'd expect the OperatorPublish$InnerProducer and below (the last four lines) to not be retained.

[Thomvis]

I've some more evidence which indicates that the operator is not properly releasing a reference to an old observable. The following screenshots were taken while stepping through the test. Please note the following:

• The field of interest currentProducer is highlighted
• currentProducer is null until just is fed to the operator, after that it contains an IterableProducer with id @1058
• The value of currentProducer does not change, even if new observables are fed to the operator. Until giving it an observable with two values, the producer remains @1058. I would expect the producer to change to the producer of the subsequent observables.
• Feeding the two-element observable finally results in a new producer (@1108)

[akarnokd]

I guess we could flush the arbiter in case the next source doesn't do backpressure and thus never calls setProducer(). However, just() does support backpressure since 1.1.1. Are you testing the latest RxJava?

[akarnokd]

Fix in #3814.

[Thomvis]

Thanks so much, this is great. Hope to see the fix in a release soon.

I was testing with 1.1.0, for no good reason other than that we've not upgraded yet. In the real app, the observable that was not getting released was followed by an Observable.empty(), not sure if that one has back pressure so that it would've made a difference.

Thanks again!

[HotIceCream]

I had similar problem. I wrote test: https://gist.github.com/HotIceCream/8010c296d6f1ad9fc6882e2dcf2d13d3 with some reflection hack.

I build library from 3814 pull request but test still not passed. Is it different bug? Or it isn't it correct test?

[akarnokd]

@HotIceCream your sources never complete and keep the subscriber active.

[akarnokd]

@HotIceCream your sources never complete and keep the subscriber active.

[HotIceCream]

@akarnokd, yes. But when i use subjects it is correct behavior. I waiting new elements from subject until switchMap called. And when switch map called, from previous observable should producer removed.

[akarnokd]

You are calling cache on the same source you are testing; the switchMap just keeps resubscribing to o and thus you always have 1 subscriber to it.

[HotIceCream]

@akarnokd, yes. It is what i want. But inside CachedObservable after each resubscribing in array of producers adding new ReplayProduecer. And it isn't removed until o called onComplete (i.e. if using subjects - never). Test create 100 ReplayProduecers.
May be it is separate bug?

[akarnokd]

No, its the misunderstanding of the lifecycle of cache(). If you do this without the switchMap, you'll get 100 subscribers in the subject as well:

PublishSubject<Object> ps = PublishSubject.create();

for (int i = 0; i < 100; i++) {
    ps.cache().subscribe().unsubscribe();
}

Once started, cache() can't be stopped. This was a decided tradeoff.

[akarnokd]

Closing via #3836.