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

isConnected is true in the webSocketDidDisconnect delegate mess #147

Closed
ghost opened this issue Dec 11, 2015 · 2 comments
Closed

isConnected is true in the webSocketDidDisconnect delegate mess #147

ghost opened this issue Dec 11, 2015 · 2 comments

Comments

@ghost
Copy link

ghost commented Dec 11, 2015

WebSocket's isConnected property is still true when the webSocketDidDisconnect delegate message is received by the client. This is kind of weird since the name implies the disconnection already happened.

It looks like 4c4495d changed it so the internal connected property wasn't set to false until after the delegate message was sent, and only refers to doDisconnect. But HEAD's implementation doesn't seem to reference that at all.

Just wondering if there's any design considerations I'm missing here or if it seems reasonable to set that property to false before calling the delegate. I'll send a PR if there are no objections.

@daltoniam
Copy link
Owner

Apologizes on the delayed response, was on vacation. I have had a lot of back and forth with this, but I agree you are right, it should be set to false when didDisconnect is called and true when didConnect is called. Only thing to mention is the delegate is called from a different thread then the connected property, so a possible race condition has to be handled. PR is welcomed. Thanks!

@daltoniam
Copy link
Owner

The 1.1.0 release should resolve this. Let us know if you have any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant