Skip to content

Re-enable ping timeout behaviour for 3x branch #351

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.

Sign up for GitHub

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

Merged
jmigual merged 4 commits into from
Mar 9, 2022
Merged

Re-enable ping timeout behaviour for 3x branch #351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
46c546e
Re-enable ping timeout
TorMFinn Mar 8, 2022
5a7752b
Removed cout statements
TorMFinn Mar 9, 2022
ee28bb6
Merge branch 'socketio:master' into master
TorMFinn Mar 9, 2022
22aa96a
change from pong to ping log
TorMFinn Mar 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 19 additions & 36 deletions src/internal/sio_client_impl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,37 +303,14 @@ namespace sio
}
}

void client_impl::ping(const asio::error_code& ec)
{
if(ec || m_con.expired())
{
if (ec != asio::error::operation_aborted)
LOG("ping exit,con is expired?"<<m_con.expired()<<",ec:"<<ec.message()<<endl);
return;
}
packet p(packet::frame_ping);
m_packet_mgr.encode(p, [&](bool /*isBin*/,shared_ptr<const string> payload)
{
lib::error_code ec;
this->m_client.send(this->m_con, *payload, frame::opcode::text, ec);
});
if(!m_ping_timeout_timer)
{
m_ping_timeout_timer.reset(new asio::steady_timer(m_client.get_io_service()));
std::error_code timeout_ec;
m_ping_timeout_timer->expires_from_now(milliseconds(m_ping_timeout), timeout_ec);
m_ping_timeout_timer->async_wait(std::bind(&client_impl::timeout_pong, this, std::placeholders::_1));
}
}

void client_impl::timeout_pong(const asio::error_code &ec)
void client_impl::timeout_ping(const asio::error_code &ec)
{
if(ec)
{
return;
}
LOG("Pong timeout"<<endl);
m_client.get_io_service().dispatch(std::bind(&client_impl::close_impl, this,close::status::policy_violation,"Pong timeout"));
LOG("Ping timeout"<<endl);
m_client.get_io_service().dispatch(std::bind(&client_impl::close_impl, this,close::status::policy_violation,"Ping timeout"));
}

void client_impl::timeout_reconnect(asio::error_code const& ec)
Expand Down Expand Up @@ -484,11 +461,6 @@ namespace sio

void client_impl::on_message(connection_hdl, client_type::message_ptr msg)
{
if (m_ping_timeout_timer) {
asio::error_code ec;
m_ping_timeout_timer->expires_from_now(milliseconds(m_ping_timeout),ec);
m_ping_timeout_timer->async_wait(std::bind(&client_impl::timeout_pong, this, std::placeholders::_1));
}
// Parse the incoming message according to socket.IO rules
Comment on lines 463 to 464
Copy link
Collaborator

@jmigual jmigual Mar 9, 2022

Choose a reason for hiding this comment

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

Shouldn't you call update_ping_timeout_timer() here?

Suggested change
{
if (m_ping_timeout_timer) {
asio::error_code ec;
m_ping_timeout_timer->expires_from_now(milliseconds(m_ping_timeout),ec);
m_ping_timeout_timer->async_wait(std::bind(&client_impl::timeout_pong, this, std::placeholders::_1));
}
// Parse the incoming message according to socket.IO rules
{
update_ping_timeout_timer()
// Parse the incoming message according to socket.IO rules

Copy link
Contributor Author

@TorMFinn TorMFinn Mar 9, 2022

Choose a reason for hiding this comment

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

I could re-add this, but I'm not sure it's "correct" behaviour to essentially treat every received message from the server as a "ping". I have checked the client implementation for Java script, and it seems like that client only updates the timer on a ping package also.

https://github.com/socketio/socket.io-client/blob/master/dist/socket.io.js

Copy link
Collaborator

@jmigual jmigual Mar 9, 2022

Choose a reason for hiding this comment

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

You are right. The server is the one periodically sending ping packets that the client must reply. For more info see: https://socket.io/docs/v4/how-it-works/#disconnection-detection

m_packet_mgr.put_payload(msg->get_payload());
}
Expand Down Expand Up @@ -525,6 +497,9 @@ namespace sio
m_ping_timeout = 60000;
}

// Start ping timeout
update_ping_timeout_timer();

return;
}
failed:
Expand All @@ -534,17 +509,15 @@ namespace sio

void client_impl::on_ping()
{
// Reply with pong packet.
packet p(packet::frame_pong);
m_packet_mgr.encode(p, [&](bool /*isBin*/,shared_ptr<const string> payload)
{
this->m_client.send(this->m_con, *payload, frame::opcode::text);
});

if(m_ping_timeout_timer)
{
m_ping_timeout_timer->cancel();
m_ping_timeout_timer.reset();
}
// Reset the ping timeout.
update_ping_timeout_timer();
}

void client_impl::on_decode(packet const& p)
Expand Down Expand Up @@ -589,6 +562,16 @@ namespace sio
m_ping_timeout_timer.reset();
}
}

void client_impl::update_ping_timeout_timer() {
if (!m_ping_timeout_timer) {
m_ping_timeout_timer = std::unique_ptr<asio::steady_timer>(new asio::steady_timer(get_io_service()));
}

asio::error_code ec;
m_ping_timeout_timer->expires_from_now(milliseconds(m_ping_interval + m_ping_timeout), ec);
m_ping_timeout_timer->async_wait(std::bind(&client_impl::timeout_ping, this, std::placeholders::_1));
}

void client_impl::reset_states()
{
Expand Down
4 changes: 3 additions & 1 deletion src/internal/sio_client_impl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ namespace sio

void ping(const asio::error_code& ec);

void timeout_pong(const asio::error_code& ec);
void timeout_ping(const asio::error_code& ec);

void timeout_reconnect(asio::error_code const& ec);

Expand Down Expand Up @@ -181,6 +181,8 @@ namespace sio
void reset_states();

void clear_timers();

void update_ping_timeout_timer();

#if SIO_TLS
typedef websocketpp::lib::shared_ptr<asio::ssl::context> context_ptr;
Expand Down