fix(share): Prevent setup/reset race condition in share with refCount

[henry-alakazhang]

Description: Fixes the bug described in #6760, which seems to be a race condition.

The conditions are as such:

• Have an observable that emits immediately, but does not complete (eg. a BehaviorSubject, or something with startWith)
• Share it with shareReplay (or any share with refCount: true)
• Subscribe multiple times simultaneously, then unsubscribe immediately (can happen with firstValueFrom)
• The shared observable then gets into a bad state where it stays subscribed to the source observable but doesn't emit new values when next subscribed to.

The reason this happens is because share adds the reset subscription before subscribing to the source. As a result, if we subscribe and unsubscribe simultaneously, the reset block can actually run before subscribing to the source. When it gets reset, the connection state gets reset as well, and it subscribes to the source again even though it's already been reset.

In the PR, I've fixed it via adding an extra check before subscribing to make sure we actually need to subscribe to the source. I feel like this is only patches over the issue and some other edge case may still be broken, but there are some comments in the code saying that the reset subscription needs to be before the source subscription, so I didn't want to move anything else around.

I added a test as well, though it tests a symptom (an excess subscription) rather than the real impact (doesn't emit again later). Couldn't get the latter working.

BREAKING CHANGE: No

Related issue (if exists): #6760

[jkossis]

@benlesh curious what your thoughts are on this. It looks like this may have cropped up with the simplification of multicast operators in 7.x.

[jkossis]

@benlesh I noticed your comment from earlier (perhaps it was deleted?), but I am not 100% convinced this is a bug with combineLatest. Or at least, that may be a separate bug. If you look through the linked issue, this race behavior can be elicited without the use of combineLatest.

In my case, I noticed it while using shareReplay on the server-side for broadcasting events to clients over gql subscriptions. Clients that connect/disconnect around the same time can run into this behavior, where it gets stuck (nothing is emitted to the "late" subscriber).

[benlesh]

Yeah. Exploring this, I see the issue. It also looks like it's time for me to clean up share (again) 😩 . I experimented with it a bit, and I think this is the best fix for now. Thank you.

[jkossis]

@benlesh is there any way to get a minor/patch publish for including this change on 7.x?

EDIT
Just noticed this commit in your fork. I suppose you'd want to get that merged before considering a version bump.