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

Add automatic ping on websockets #947

Merged
merged 4 commits into from
Jan 15, 2015
Merged

Conversation

lultimouomo
Copy link
Contributor

The ping interval is fixed at 60 seconds.
The best thing would probably be to set it in the server settings, thus also allowing to disable it entirely.
For this, the WebSocket must have access to HTTPServerSettings; the best way would probably be to expose a const reference to HTTPServerRequest.m_settings, which I'm going to do in the next commit of this pull request.

@lultimouomo lultimouomo force-pushed the pingPong branch 2 times, most recently from a06f96d to 562954b Compare December 31, 2014 13:56
@lultimouomo
Copy link
Contributor Author

Apparently older versions of std.conv.to do not support dynamic-to-static array conversion; I've now made the conversion manually to support older toolchains.

@property const(HTTPServerSettings) serverSettings() const
{
return m_settings;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave this as package for now to avoid any potential necessity for API breakage later. If it turns out that this is useful for external libraries/applications, we can still make it public.

Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
It will be made accessible to the websocket via HTTPServerRequest

Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
@s-ludwig
Copy link
Member

Looks good to merge, except for the public serverSettings property.

Makes a const reference to HTTPServerSettings accessible to HTTP
delegates.

Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
Signed-off-by: Luca Niccoli <l.niccoli@awtech.it>
@lultimouomo
Copy link
Contributor Author

I made the settings accessor package, and I fixed a bug that came out testing ping timeouts:
the m_pingTimer task was trying to directly close the connection, which was most probably acquired by m_reader. The code now makes m_reader close the connection itself, like the other code paths that lead to the connection being closed.

@s-ludwig
Copy link
Member

Great, thanks! I'll merge as soon as the Travis build is ready.

s-ludwig added a commit that referenced this pull request Jan 15, 2015
Add automatic ping on websockets
@s-ludwig s-ludwig merged commit 2303f1c into vibe-d:master Jan 15, 2015
@lultimouomo lultimouomo deleted the pingPong branch January 15, 2015 10:20
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

Successfully merging this pull request may close these issues.

None yet

3 participants