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

Subscription callback is not nulled out when Subscription is unsubscribed #5464

Closed
felixfbecker opened this issue Jun 1, 2020 · 0 comments · Fixed by #5465, vmware-archive/octant#2113 or TNG/ApiCenter#630 · May be fixed by nuel/mastodon-old#670
Labels
bug Confirmed bug

Comments

@felixfbecker
Copy link
Contributor

felixfbecker commented Jun 1, 2020

Bug Report

Current Behavior
When instantiating a Subscription and passing a callback, that callback is saved as this._unsubscribe of the Subscription object:

(<any> this)._unsubscribe = unsubscribe;

When unsubscribing that Subscription, the function is called, but not nulled out afterwards:

_unsubscribe.call(this);

This means that if it did some manual resource cleanup like closing a MessagePort, that MessagePort still can't be garbage-collected, because the callback, and therefor the Subscription, still holds a reference to it.

This is different from Subscriptions added via add(), which are correctly de-referenced:

this._subscriptions = null;

Reproduction

const { port1 } = new MessageChannel()
const subscription = new Subscription(() => {
  port1.close()
})
subscription.unsubscribe()
console.log(subscription._unsubscribe)

Expected behavior
After calling the callback, this._unsubscribe should be nulled out so the callback and its references can be garbage-collected. Since a Subscription can only be unsubscribed once, there is no need to keep the reference.

Environment

  • Runtime: All
  • RxJS version: 6.5.5

Possible Solution
Set this._unsubscribe to null when unsubscribe() is called.

@cartant cartant added the bug Confirmed bug label Jun 1, 2020
felixfbecker added a commit to felixfbecker/rxjs that referenced this issue Jun 1, 2020
cartant pushed a commit to felixfbecker/rxjs that referenced this issue Jul 26, 2020
benlesh pushed a commit that referenced this issue Jul 30, 2020
…mpotent (#5465)

* null out _unsubscribe after unsubscription

Fixes #5464

* test: add idempotent unsubscribe tests

* fix: null the subscription ctor teardown

* refactor: use delete to remove the member

* refactor: delete _unsubscribe only if it exists

* refactor: replace delete with hasOwnProperty etc

* refactor: use _ctorUnsubscribe flag

Co-authored-by: Nicholas Jamieson <nicholas@cartant.com>
benlesh pushed a commit that referenced this issue Jul 30, 2020
…mpotent (#5465)

* null out _unsubscribe after unsubscription

Fixes #5464

* test: add idempotent unsubscribe tests

* fix: null the subscription ctor teardown

* refactor: use delete to remove the member

* refactor: delete _unsubscribe only if it exists

* refactor: replace delete with hasOwnProperty etc

* refactor: use _ctorUnsubscribe flag

Co-authored-by: Nicholas Jamieson <nicholas@cartant.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment