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

Ping timeout is canceled after the pong handler #901

Closed
oleh-derevenko opened this issue May 25, 2020 · 1 comment
Closed

Ping timeout is canceled after the pong handler #901

oleh-derevenko opened this issue May 25, 2020 · 1 comment
Milestone

Comments

@oleh-derevenko
Copy link

In impl/connection_impl.hpp line 1974 there is ping timeout canceled after pong handler is called.

    if (m_pong_handler) {
        m_pong_handler(m_connection_hdl, msg->get_payload());
    }
    if (m_ping_timer) { // <------------------------------ HERE
        m_ping_timer->cancel();
    }

The order of these blocks is obviously wrong.
Since m_pong_handler() is an invocation of external code (with respect to current class) the class must not assume anything about the duration the handler may take. This way it is logical to first cancel the timeout timer (since the operation has succeeded already) and only then start calling an external handler of unknown duration.

@zaphoyd
Copy link
Owner

zaphoyd commented Jun 7, 2020

This makes sense. I think in practice serialization of handlers should mitigate most adverse effects, but reversing the order here makes more logical sense and should be more robust against exceptions and avoid doing any unnecessary work.

@zaphoyd zaphoyd added this to the 0.9.0 milestone Aug 7, 2020
TwentyPast4 pushed a commit to TroniusGaming/websocketpp that referenced this issue Aug 18, 2022
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

2 participants