Skip to content

OperatorReplay leaks its subscribers #4228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ntoskrnl opened this issue Jul 22, 2016 · 6 comments
Closed

OperatorReplay leaks its subscribers #4228

ntoskrnl opened this issue Jul 22, 2016 · 6 comments
Labels

Comments

@ntoskrnl
Copy link

ntoskrnl commented Jul 22, 2016

We use replay(1) a lot in our Android projects. All subscriptions are kept in CompositeSubscription and, of course, we always clear it when we don't need it anymore.

Even though we unsubscribe from observable, subscriber won't get collected by GC, because it is referenced by OperatorReplay, which prevents Activity or Fragment from being garbage collected. We do need to keep observable alive and resubscribe to it from new Activity instance, but now we can't use replay() as all our Activities are leaked.

Right now we downgraded back to 1.1.5. Alternatively, one can use publish() with behavior subject to work around this issue. I believe the issue was introduced in one of the latest versions of the library (perhaps 1.1.6, not sure though).

@akarnokd
Copy link
Member

Do you have your replay in a completed state or with infrequent updates?

@akarnokd akarnokd added the Bug label Jul 22, 2016
@ntoskrnl
Copy link
Author

Infinite stream is "replayed", so, I guess, it is never in a completed state.
What do you mean by infrequent updates?

@ntoskrnl
Copy link
Author

ntoskrnl commented Jul 22, 2016

Connectable observable with replay example (no error handling to keep it simple):

val dataStream = refreshes.startWith(Unit)
        .flatMap { model.getData() }
        .replay(1)

When presenter is created we connect to the stream and store subscription until presenter is destroyed:

val presenterSubscription = CompositeSubscription()
val viewSubscription = CompositeSubscription()

presenterSubscription.add(dataStream.connect())

View is attached to presenter:

// subscribe view to the data stream, subscriber has a strong reference to view
viewSubscription.add(dataStream.subscribe {  view.showContent(it) })

View detached from presenter:

// clear view subscription
// after this there should be no strong references to the view
viewSubscription.clear()

When presenter is destroyed (when activity is closed):

presenterSubscription.unsubscribe()

Throughout the presenter lifecycle, different instances of view are attached and then detached (for instance, during screen rotation when Activity is recreated). However, after the new view instance is attached, the old instance is still referenced from within OperatorReplay. According to heap dump analysis, each time there exist only two instances of the view – no matter how I many times I rotate the screen, there are two instances of view: current and only 1 leaked view.

@ntoskrnl
Copy link
Author

👍

@akarnokd
Copy link
Member

Thanks for the report. This is a bug in replay()'s internal cache of Subscribers. I've posted a PR #4229 with the fix. A temporary workaround is to use onTerminateDetach on the ConnectableObservable the replay() returns and subscribe through it.

@akarnokd
Copy link
Member

Fixed via #4229.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants