Subscription to Observable is not removed from observers array after unsubscribe()

Bug Report

Current Behavior
Currently subscription isn't removed from observers array after unsubscribe, both with takeUntil() and unsubscribe().

Reproduction

function called() { console.log('CALLED'); }

const subject = new Subject();
const destroyed = new Subject();

const subscription = subject
        .pipe(takeUntil(destroyed))
        .subscribe(() => called());

console.log(subject.observers.length); // 1

subject.next(); // CALLED
subject.next(); // CALLED

// Unsubscribe from subject
destroyed.next(true);
destroyed.complete();

console.log(subject.observers.length); // Still 1, expected: 0

subscription.unsubscribe();
subject.next(); // no console.log('CALLED')

console.log(subject.observers.length); // Still 1, expected: 0

Expected behavior
In 2 last console.log(subject.observers.length) it's expected to have 0 : observer should be removed from array of observers after unsubscribe()

Environment

RxJS version:
6.3.3

Additional notes
It isn't reproducible with rxjs version 6.2.2

same issue here after upgrading from 6.3.2 to 6.3.3

For me, this issue's repro outputs the expected counts:

$ ts-node index.ts
1
CALLED
CALLED
0
0

The code is unchanged:

import { Subject } from "rxjs";
import { takeUntil } from "rxjs/operators";

function called() { console.log('CALLED'); }

const subject = new Subject();
const destroyed = new Subject();

const subscription = subject
  .pipe(takeUntil(destroyed))
  .subscribe(() => called());

console.log(subject.observers.length); // 1

subject.next(); // CALLED
subject.next(); // CALLED

// Unsubscribe from subject
destroyed.next(true);
destroyed.complete();

console.log(subject.observers.length); // Still 1, expected: 0

subscription.unsubscribe();
subject.next(); // no console.log('CALLED')

console.log(subject.observers.length); // Still 1, expected: 0

And I'm testing it with 6.3.3:

$ npm list rxjs
so-rxjs-scratchpad@0.0.0 C:\Users\Nicholas\git\temp\so-rxjs-v6-scratchpad
`-- rxjs@6.3.3

@cartant You are right, i've started new (angular) project and couldn't reproduce this issue with RxJS 6.3.3.
But still i can reproduce it here:
https://stackblitz.com/edit/rxjs-me3evv?file=index.ts

I've copied content of node_modules/rxjs/bundles/rxjs.umd.js bundle and put it into file rxjsbundle.js in my example on stackblitz.

Also, some additional note: i found this issue in my angular project's test and when i start this karma test with ng - it's reproducible. I've updated only rxjs in my package.json (from 6.2.2 to 6.3.3 as i mentioned previously) - and that failed.

Thanks for your time and would be nice if you could clarify this.

So are you saying that it's only reproducible when using the bundle that's in 6.3.3?

Actually i've just tried to copy bundle's content to my example from versions 6.3.2, 6.3.1 and even from 6.2.2 - it's still reproducible with stackblitz and that's pretty strange.
But the test got failed only with RxJS version 6.3.3 and is working with 6.3.2

This works fine for me, too:

<html>
<body>
<script src="./node_modules/rxjs/bundles/rxjs.umd.js"></script>
<script>

const { Subject } = rxjs;
const { takeUntil } = rxjs.operators;

function called() { console.log('CALLED'); }

const subject = new Subject();
const destroyed = new Subject();

const subscription = subject
    .pipe(takeUntil(destroyed))
    .subscribe(() => called());

console.log(subject.observers.length); // 1

subject.next(); // CALLED
subject.next(); // CALLED

// Unsubscribe from subject
destroyed.next(true);
destroyed.complete();

console.log(subject.observers.length); // Still 1

subscription.unsubscribe();
subject.next(); // no console.log('CALLED')

console.log(subject.observers.length); // Still 1

</script>
</body>
</html>

In your StackBlitz, you are using two separate implementations of RxJS: the bundle; and the package.json dependency. You are importing Subject from the former and takeUntil from the latter.

I suspect that the problem is directly related to the two implementations and that the changes made in 6.3.3 that have effected this behaviour are in 50ee0a7 and/or 0972c56.

The problem is reproducible with this code:

const { Subject } = require("rxjs/bundles/rxjs.umd");
import { takeUntil } from "rxjs/operators";

function called() { console.log('CALLED'); }

const subject = new Subject();
const destroyed = new Subject();

const subscription = subject
  .pipe(takeUntil(destroyed))
  .subscribe(() => called());

console.log(subject.observers.length); // 1

subject.next(); // CALLED
subject.next(); // CALLED

// Unsubscribe from subject
destroyed.next(true);
destroyed.complete();

console.log(subject.observers.length); // Still 1, expected: 0

subscription.unsubscribe();
subject.next(); // no console.log('CALLED')

console.log(subject.observers.length); // Still 1, expected: 0

And the problem does not occur if 6.3.2 is used.

In my case I have a working angular app (cli version 6.2.4, with rxjs 6.3.2) , in which I have a component behaving more or less like this:

import { Component, OnDestroy, OnInit } from '@angular/core';
import { Subscription } from 'rxjs';
import { tap } from 'rxjs/internal/operators';

export class MyComponent implements OnInit, OnDestroy {
    myObservable: Observable<T>;
    mySubscription: Subscription;

    ngOnInit() {
        console.log('subscription');
        this.mySubscription = this.myObservable.pipe(
            tap(() => {
                console.log('do stuff with observable');
            })
        ).subscribe(
            () => {
                console.log('do other stuff!');
            }
        );
    }

    ngOnDestroy() {
        console.log('unsubscribe');
        this.mySubscription.unsubscribe();
     }
}

After the update to rxjs 6.3.3, in the console I see:
> subscription
> do stuff with observable
> do other stuff!
> unsubscribe
> do stuff with observable

@mdepasquale21 For this problem to manifest, I think you'd need to have multiple copies of RxJS installed in node_modules - either different versions or the same version in different locations.

Run npm list rxjs in your project's root to see what's installed.

@cartant you were right, shame on me! Some other dependencies in my app (@angular-devkit), which depend in turn internally on rxjs, downloaded version 6.2.2 when npm installed, even if in their package.json they have rxjs: ''~6.3.0" (https://github.com/angular/angular-cli/blob/master/package.json). I tried also to delete node_modules and re-install everything, but looks like nothing changed. rxjs in my package.json was updated to 6.3.3, but version in @angular/cli was 6.2.2 so it was not the same everywhere, as you suggested. For some reason until version 6.3.2 this mixture did not break stuff!
I've tried a few solutions and it seems to me that it is very difficult to "force" the download of rxjs 6.3.3 for all dependencies, so actually I'm reverting to rxjs 6.2.2 to make everything work again. But I remain interested in future developments of this post!
Thanks!

Pretty sure this defect resulted in a huge memory leak across a bunch of our angular sites.

Reverting to rxjs 6.2.2 resolved the issue for us.

hi all - i am still a bit confused as to why there would be different version of rxjs installed for an angular application, especially when rxjs is listed as a peerDependency. I did a build w/o optimzation to check to see if there were multiple versions of rxjs installed and couldn't find more than 1. Also, when i run npm list rxjs, the version returned is 6.2.2; however when i look inside ./node_modules, I can clearly see that 6.3.3 is installed. I guess I would expect npm list rxjs to return 6.3.3? Admittedly, this is more of a how does npm work question rather than a rxjs question, but hoping someone here could enlighten me since it's related :). Thanks all!

Same isssue here since i have update from 6.3.2 to 6.3.3 with angular-cli

@danielgeri I'm not sure either, the only thing I can suggest is that you try to delete node_modules directory and run npm install to reinstall everything from scratch. In my case when I tried this solution it didn't work. I had rxjs listed as a peer dependency but other libraries in my package.json which depend on rxjs for some reason were using a different version. For me it was completely unexpected, I had to revert to rxjs 6.2.2. Anyway after updating to Angular 7 everything works fine and I could finally use rxjs 6.3.3.

I tried everything with deleting node_modules directory.

Angular 6 and rxjs 6.3.3 => not working
Angular 6 and rxjs 6.3.2 => working

Angular 7 and rxjs 6.3.3 => not working
Angular 7 and rxjs 6.3.2 => working

FWIW, I've had the same issue when upgrading from 6.3.2 to 6.3.3, and the bug was in fact caused by an incorrect import: I imported from 'rxjs/internal/operators' instead of 'rxjs/operators'.

@jnizet Do you have a minimal reproduction? I have an idea of how using the internal import location could be problematic, but I'd like to reproduce the problem. Mostly because I'm curious.

I don't have a public one. I'll try creating one if I have some time, but it won't be now.

May be related with the discussion, I have an update with respect to my previous answer and in light of @jnizet 's answer: with angular 7 and rxjs 6.3.3 I thought everything was working fine, but instead I had a huge, random bug on initialization of the app! Sometimes the app just did not start, loading forever while sometimes started normally with no errors. So this 'init' bug appeared randomly without apparent reason whatsoever and it was difficult to reproduce and therefore to solve. I noticed I had several imports from 'rxjs/internal/operators' and changed them to import from 'rxjs/operators' as @jnizet suggested. This solved the problem finally (or at least it looks like, the bug has never appeared again since then).

I have the same issue.
I downgraded to rxjs 6.3.2 and now is working.

it looks like it work well with the new version 6.4.0. If someone can confirm ;)

This issue is related to the pipe() operator - removing the operator before subscribe() restores functionality of the Subscription's object unsubscribe method.

I had this issue in combination with Angular 7. Using different versions (6.3.3 or 6.4.0 ...) had no effect. Angular versions are predeterming the rxjs version anyway.

@BenjaminBuick If you have one, a reproduction of the problem would be appreciated. The comment you have left is unclear and not actionable, IMO.

the code:

subscription: Subscription;

constructor() {
this.subscription = this.contarTresInfinito().subscribe(
(response: any) => {
console.log('respuesta', response);
},
(error) => {
console.error('hay error', error);

  }).add(
    () => {
      console.log('terminó')
  });

}

ngOnDestroy() {
this.subscription.unsubscribe();
console.log ('la pagina se va a cerrar');
}

public contarTresInfinito(): Observable {

return new Observable((observer: Subscriber<any>) => {

  let contador = 0;
  let intervalo = setInterval(() => {

    contador++;

    const salida = {
      valor: contador
    }

    observer.next(salida); //esto es lo que notifica

  }, 1000);

}).pipe(
  map(response => {
    return response.valor;
  }),
  filter((response, index) => { //el filter siempre devuelve un bool, el primer valor es el response y el segundo la cantidad de veces que es llamado

    if ((response % 2) == 1) {
      //impar
      return true;
    } else {
      //par
      return false;
    }
  })
);

it never unsubscribe.

well, i update the rxjs to 6.4.0, and then removed this line of the code
.add(
() => {
console.log('terminó')
});

now works perfect.

I use angular 7.2.0 and rxjs 6.3.3 My problem is like this below. unsubscribe() has no effect and this.subscription.unsubscribe() in ngOnDestroy(){}.
is this problem persists?

I use angular 7.2.0 and rxjs 6.3.3 My problem is like this below. unsubscribe() has no effect and this.subscription.unsubscribe() in ngOnDestroy(){}.
is this problem persists?

do you have this line? .add() at the end of your subscribe?

We faced the same issue with rxjs 6.3.3. After reading through all the comments here, we discovered that there was indeed a bad import of switchMap from rxjs/internal/operators. After fixing this import, everything works as expected.

With the absence of minimal reproduction - and with reports that the problem is due to importing an internal file - IMO, this issue should just be closed.

I also started encountering this after upgrading from Angular 6 to 7. On Chrome everything would behave, but on Firefox I would observe the following as the app would initialize.

Unhandled Promise rejection: You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable. ; Zone: ; Task: Promise.then ; Value: TypeError: "You provided an invalid object where a stream was expected. You can provide an Observable, Promise, Array, or Iterable."

I tried different versions of rxjs, examining closely npm list, etc - all to no avail.

Finally what did the trick was to change
import {Subject} from 'rxjs/index'; //This did not cause issues before the upgrade
to
import {Subject} from 'rxjs';

Crazy stuff!

It seems like import statements from 'rxjs/index' and 'rxjs/internal/*' (and perhaps others) should cause compilation errors.

@PaulAGH I agree. This almost drove me mad :)

It seems like import statements from 'rxjs/index' and 'rxjs/internal/*' (and perhaps others) should cause compilation errors.

I've added an rxjs-no-index rule to https://github.com/cartant/rxjs-tslint-rules/ and a no-index rule to https://github.com/cartant/eslint-plugin-rxjs. I would recommend using one of those rules to guard against explicit importation from an index module.

I've made numerous requests for a minimal reproduction and - unless I've missed something - none has been provided. So this is not actionable, IMO.

It's possibly related to what #5059 will fix, but without a repro ... 🤷‍♂

I've made numerous requests for a minimal reproduction and - unless I've missed something - none has been provided. So this is not actionable, IMO.

It's possibly related to what #5059 will fix, but without a repro ... 🤷‍♂

I questioned closing the issue on false premise. Sorry about confusion.

Im having the same issue. looking at the code I wonder why

rxjs/src/internal/Subscriber.ts

Lines 130 to 136 in 06f1a25

    
           unsubscribe(): void { 
        
             if (this.closed) { 
        
               return; 
        
             } 
        
             this.isStopped = true; 
        
             super.unsubscribe(); 
        
           }

does not this.destination.unsubscribe()

As I've said above:

I've made numerous requests for a minimal reproduction and - unless I've missed something - none has been provided. So this is not actionable, IMO.

Seriously, no one is going to be able to answer any questions about this without a minimal reproduction of the problem.

Any news?

Closing this because it cannot be reproduced.

	unsubscribe(): void {
	if (this.closed) {
	return;
	}
	this.isStopped = true;
	super.unsubscribe();
	}