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

Fix multiple concurrent websocket connects failing #2265

Merged
merged 10 commits into from Feb 18, 2019

Conversation

@s-ludwig
Copy link
Member

@s-ludwig s-ludwig commented Feb 17, 2019

Fixes #2169. This is a full solution that also fixes a socket descriptor leak and supercedes the workaround in #2172.

s-ludwig added 4 commits Feb 17, 2019
- The connection is now only closed by the main request loop to avoid stale connection references
- switchProtocol sets a flag to ensure that the connection is not getting reused for further requests
This change required a rework of how the connection close and resource management is done to avoid InvalidMemoryOperationError. As a side-effect, this also fixes a leak of TCP connections for connectWebSocket.

The rework in particular makes sure that HTTPClientResponse.disconnect and HTTPServerResponse.finalize is called, and only called from the task that created the web socket. This makes sure that the HTTP client instance is properly returned to the connection pool after the web socket is closed.

Closes #2172.
s-ludwig added 3 commits Feb 18, 2019
HTTPClient.disconnect already destroys the connection, which means that the `.connected` call in `WebSocket.close` accesses a destroyed memory region.
@dlang-bot dlang-bot merged commit 4612d8b into master Feb 18, 2019
3 checks passed
@s-ludwig s-ludwig deleted the issue_2169_multiple_websocket_fail branch Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants