forkJoin doesn't wait for last input's finally block

[deastr]

RxJS version: 6.5.2

Additional information:

forkJoin doesn't seem to wait for the last input's finally block when the input observables have finally blocks.

Code to reproduce:

  get(url: string): Observable<any> {
    return this.http.get(url).finally(() => console.log(url + ' completed.'));
  }

  bulkRequest(...sources: SubscribableOrPromise<any>[]): Observable<any[]> {
    console.log('bulkRequest start'); 
    return forkJoin(sources)
        .finally(() => console.log("forkjoin completed"));
  }

  ngAfterViewInit(){
    this.bulkRequest(this.get('https://jsonplaceholder.typicode.com/posts/1'),
    this.get('https://jsonplaceholder.typicode.com/posts/2'),
    this.get('https://jsonplaceholder.typicode.com/posts/3'),
    this.get('https://jsonplaceholder.typicode.com/posts/4'),
    this.get('https://jsonplaceholder.typicode.com/posts/5'),
    this.get('https://jsonplaceholder.typicode.com/posts/6'),
    this.get('https://jsonplaceholder.typicode.com/posts/7'),
    this.get('https://jsonplaceholder.typicode.com/posts/8'),
    this.get('https://jsonplaceholder.typicode.com/posts/9'),
    this.get('https://jsonplaceholder.typicode.com/posts/10'))
    .subscribe(results => {
      console.log('bulkRequest completed');
      for(let i = 0; i < results.length; i++) {
        console.log('result ' + (i + 1).toString() + ': ' + results[i].id);
      }
    });
  }

https://stackblitz.com/edit/angular-u3vqnu

Expected behavior:

bulkRequest start
https://jsonplaceholder.typicode.com/posts/1 completed.
https://jsonplaceholder.typicode.com/posts/2 completed.
https://jsonplaceholder.typicode.com/posts/3 completed.
https://jsonplaceholder.typicode.com/posts/4 completed.
https://jsonplaceholder.typicode.com/posts/5 completed.
https://jsonplaceholder.typicode.com/posts/6 completed.
https://jsonplaceholder.typicode.com/posts/7 completed.
https://jsonplaceholder.typicode.com/posts/8 completed.
https://jsonplaceholder.typicode.com/posts/9 completed.
https://jsonplaceholder.typicode.com/posts/10 completed.
bulkRequest completed
result 1: 1
result 2: 2
result 3: 3
result 4: 4
result 5: 5
result 6: 6
result 7: 7
result 8: 8
result 9: 9
result 10: 10
forkjoin completed

Actual behavior:

bulkRequest start
https://jsonplaceholder.typicode.com/posts/1 completed.
https://jsonplaceholder.typicode.com/posts/2 completed.
https://jsonplaceholder.typicode.com/posts/3 completed.
https://jsonplaceholder.typicode.com/posts/4 completed.
https://jsonplaceholder.typicode.com/posts/5 completed.
https://jsonplaceholder.typicode.com/posts/6 completed.
https://jsonplaceholder.typicode.com/posts/7 completed.
https://jsonplaceholder.typicode.com/posts/8 completed.
https://jsonplaceholder.typicode.com/posts/9 completed.
bulkRequest completed
result 1: 1
result 2: 2
result 3: 3
result 4: 4
result 5: 5
result 6: 6
result 7: 7
result 8: 8
result 9: 9
result 10: 10
forkjoin completed
https://jsonplaceholder.typicode.com/posts/10 completed.

The last "completed" one isn't always necesserily the 10th one, any other could be.

[cartant]

This does not look like a bug, to me. The first thing to note is that the code logs bulkRequest completed from within the handler for a next notification and that notification will be emitted before completion.

[deastr]

So is this the order of execution:

1. last http.get()
2. bulkRequest.subscribe (which is forkJoin.subscribe I think?)
3. forkJoin.finally
4. last http.get().finally

[benlesh]

const source = forkJoin(
  [1, 2, 3, 4].map(n =>
    timer(100).pipe(
      map(() => n),
      finalize(() => console.log(`Finalized ${n}`))
    )
  )
);

source.subscribe(console.log);

Output:

Finalized 1
Finalized 2
Finalized 3
[1, 2, 3, 4]
Finalized 4

It's certainly surprising behavior, I guess. I mean, I know why it's happening, but I think it would be best if it were more predictable. IMO, this is a bug similar to some other issues I've fixed recently. And I can think of other places this is probably happening too.

[cartant]

but I think it would be best if it were more predictable

FWIW, this predictability - i.e. a simple/small set of rules - is one of the things I had in mind when we were discussing the take business. I really do think that all consumers should unsubscribe ASAP - i.e. before sending further notifications - but I don't think we can do this before v8. I mean, we can fix this issue, but I'm pretty sure it will pop up elsewhere and we still won't have a simple rule to reason with.