Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

Autoconnect in the background #12

Closed
KernelPryanic opened this issue Mar 22, 2018 · 3 comments
Closed

Autoconnect in the background #12

KernelPryanic opened this issue Mar 22, 2018 · 3 comments
Assignees
Labels
client feature request (deprecated) This label is deprecated since December 10, 2018
Milestone

Comments

@KernelPryanic
Copy link
Collaborator

Issue #11 already covers the autoconnect feature but misses one important case:

When the client connects to the server just to listen to server-side signals - the connection could get lost at some point (due to a server reboot for example) and the user-code will assume, that there's just nothing happening on the server, but in reality the server just won't send the signals due to the client being disconnected whole the time.
In such a case it's very important for the client to try to reconnect in the background as soon as the connection is lost to ensure maximum uptime.

@romshark romshark self-assigned this Mar 22, 2018
@romshark romshark added this to the v1.0.0 rc1 milestone Mar 22, 2018
@romshark
Copy link
Owner

romshark commented Mar 23, 2018

I've successfuly implemented this feature in the background-autoconnect branch, though faced a critical bug, getting a 1002 (protocol error): unexpected reserved bits 0x50 causing the server to close the reestablished connection for unknown reasons. Seems like some invalid state is left somewhere in the underlying websocket dialer causing the reestablished connection to send corrupted messages. I've posted an issue about it.

@romshark
Copy link
Owner

romshark commented Mar 23, 2018

As garyburd denoted in the issue, the problem is caused on our side by a race condition. I checked the code with the race detector enabled and realized the client is stupid enough to neither reuse nor kill the socket-reading goroutine that is created in the client.connect method, a second reader is spawned on reconnection which finally causes unsynchronized concurrent access to the sockets ReadMessage method, gonna fix that immediately.

@romshark
Copy link
Owner

As of commit #f0ab6a3 the client will automatically try to connect in the background whenever the connection to the server is lost, as well as when the client instance is created by the constructor method wwrclt.NewClient, when the autoconnect option is enabled.

When connection loss is detected, the client will create a dedicated reconnection goroutine that's suspended when the connection is reestablished. Subsequent autoconnection attempts will wait for this goroutine to complete and possibly timeout. This way resource usage and network load is minimized.

@romshark romshark added client feature request (deprecated) This label is deprecated since December 10, 2018 and removed feature request labels Aug 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
client feature request (deprecated) This label is deprecated since December 10, 2018
Projects
None yet
Development

No branches or pull requests

2 participants