Skip to content
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

Observe on leak #2248

Merged
merged 2 commits into from
Jan 4, 2017
Merged

Observe on leak #2248

merged 2 commits into from
Jan 4, 2017

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Jan 4, 2017

  • ensures child subscriptions created via Subscription.prototype.add remove themselves from the internal subscriptions list.
  • ensures scheduled notifications from observeOn remove themselves from the subscription by calling unsubscribe on themselves.

fixes #2244

…lf when unsubscribed

There was a problem where subscriptions added to parent subscriptions would not remove themselves when unsubscribed, This meant that when Actions (which are subscriptions) were unsubscribed, they would still hang out in the _subscriptions list and live in memory when we didn't want them to

related #2244
@@ -54,15 +54,13 @@ export class VirtualAction<T> extends AsyncAction<T> {
}

public schedule(state?: T, delay: number = 0): Subscription {
return !this.id ?
super.schedule(state, delay) : (
// If an action is rescheduled, we save allocations by mutating its state,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Blesh can we keep this comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@trxcllnt
Copy link
Member

trxcllnt commented Jan 4, 2017

lgtm

@coveralls
Copy link

coveralls commented Jan 4, 2017

Coverage Status

Coverage increased (+0.007%) to 97.696% when pulling 9664a38 on blesh:observeOn-leak into 6922b16 on ReactiveX:master.

@benlesh benlesh merged commit f92eea7 into ReactiveX:master Jan 4, 2017
@josh-sachs
Copy link

Is there an estimate on when the next patch version will be available?

@benlesh
Copy link
Member Author

benlesh commented Jan 4, 2017

@kudoz very soon. This week or early next week.

@lock
Copy link

lock bot commented Jun 6, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory Leak with Queue Scheduler and Subject
4 participants