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

Unsubscribing from WebSocketSubject reference to Websocket #2037

Closed
webstp opened this issue Oct 14, 2016 · 2 comments · Fixed by #2039
Closed

Unsubscribing from WebSocketSubject reference to Websocket #2037

webstp opened this issue Oct 14, 2016 · 2 comments · Fixed by #2039
Assignees
Labels
bug Confirmed bug

Comments

@webstp
Copy link

webstp commented Oct 14, 2016

rxjs@5.0.0-rc.1

When unsubscribing from a multiplex WebSocketSubject I am seeing the socket getting set to null. There are still other subscribers and the socket still exists, but we are loosing the reference to the socket

I believe this was introduced in 62d242e.

Relevant Code

protected _subscribe(subscriber: Subscriber<T>): Subscription {
    const { source } = this;
    if (source) {
      return source.subscribe(subscriber);
    }
    if (!this.socket) {
      this._connectSocket();
    }
    let subscription = new Subscription();
    subscription.add(this._output.subscribe(subscriber));
    subscription.add(() => {
      const { socket } = this;
      if (this._output.observers.length === 0 && socket && socket.readyState === 1) {
        socket.close();
      }
      this._resetState(); //This is where to socket reference is getting lost
    });
    return subscription;
  }
@benlesh benlesh added the bug Confirmed bug label Oct 14, 2016
@benlesh
Copy link
Member

benlesh commented Oct 14, 2016

Oops, yup, I see the bug there. Thanks @webstp

@benlesh benlesh self-assigned this Oct 14, 2016
jayphelps pushed a commit that referenced this issue Oct 24, 2016
… after first unsubscribe (#2039)

This fix ensure the observers count goes to zero before the state is reset on the WebSocketSubject instance

fixes #2037
@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
bug Confirmed bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants