Skip to content

Conversation

@iakuf
Copy link

@iakuf iakuf commented Sep 8, 2021

I found that when the network is disconnected, socket.io cannot detect that the network is disconnected and will not enable reconnect. So fix this problem.

@shenX-2021
Copy link

@darrachequesne this pr is available, do you want to merge it ?

Comment on lines +550 to +554
if (m_ping_timeout_timer) {
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_pong, this, std::placeholders::_1));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you recreating the timer here instead of just cancelling it?

Copy link

Choose a reason for hiding this comment

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

When the on_ping event is received, the timer needs to be reset. If the on_ping event is not received before the next timeout, it is considered that the local network is disconnected.

Comment on lines +556 to +560
/*if(m_ping_timeout_timer)
{
m_ping_timeout_timer->cancel();
m_ping_timeout_timer.reset();
}
}*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't leave commented code. Just remove it.

Suggested change
/*if(m_ping_timeout_timer)
{
m_ping_timeout_timer->cancel();
m_ping_timeout_timer.reset();
}
}*/

Copy link

Choose a reason for hiding this comment

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

Yes, this is my problem, thanks.

@TorMFinn
Copy link
Contributor

TorMFinn commented Mar 9, 2022

I have opened another pull request that also adresses this issue.
#351

@jmigual
Copy link
Collaborator

jmigual commented Mar 9, 2022

Closing because #351 already fixed it.

@jmigual jmigual closed this Mar 9, 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

Successfully merging this pull request may close these issues.

5 participants