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

websockets: Count pings from server as activity for idle_timeout #2718

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions doc/qbk/06_websocket/06_timeouts.qbk
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ There are three timeout settings which may be set independently on the stream:
[[link beast.ref.boost__beast__websocket__stream_base__timeout.idle_timeout `timeout::idle_timeout`]]
[`duration`]
[
If no data is received from the peer for a time equal to the idle
timeout, then the connection will time out.
If no data or control frames are received from the peer for a time
equal to the idle timeout, then the connection will time out.
The value returned by
[link beast.ref.boost__beast__websocket__stream_base.none `stream_base::none()`]
may be assigned to disable this timeout.
Expand Down
12 changes: 4 additions & 8 deletions include/boost/beast/websocket/impl/stream_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -439,12 +439,8 @@ struct stream<NextLayer, deflateSupported>::impl_type
if(timeout_opt.idle_timeout != none())
{
idle_counter = 0;
if(timeout_opt.keep_alive_pings)
timer.expires_after(
timeout_opt.idle_timeout / 2);
else
timer.expires_after(
timeout_opt.idle_timeout);
timer.expires_after(
timeout_opt.idle_timeout / 2);

BOOST_ASIO_HANDLER_LOCATION((
__FILE__, __LINE__,
Expand Down Expand Up @@ -569,9 +565,9 @@ struct stream<NextLayer, deflateSupported>::impl_type
if(impl.timeout_opt.idle_timeout == none())
return;

if( impl.timeout_opt.keep_alive_pings &&
impl.idle_counter < 1)
if( impl.idle_counter < 1 )
{
if( impl.timeout_opt.keep_alive_pings )
{
BOOST_ASIO_HANDLER_LOCATION((
__FILE__, __LINE__,
Expand Down
6 changes: 4 additions & 2 deletions include/boost/beast/websocket/stream_base.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ struct stream_base
An outstanding read operation must be pending, which will
complete immediately the error @ref beast::error::timeout.

@li When `keep_alive_pings` is `false`, the connection will be closed.
An outstanding read operation must be pending, which will
@li When `keep_alive_pings` is `false`, the connection will
be closed if there has been no activity. Both websocket
message frames and control frames count as activity. An
outstanding read operation must be pending, which will
complete immediately the error @ref beast::error::timeout.
*/
bool keep_alive_pings;
Expand Down
68 changes: 68 additions & 0 deletions test/beast/websocket/ping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,74 @@ class ping_test : public websocket_test_suite
se.code().message());
}
}

// inactivity timeout doesn't happen when you get pings
{
echo_server es{log};
stream<test::stream> ws{ioc_};

// We have an inactivity timeout of 2s, but don't send pings
ws.set_option(stream_base::timeout{
stream_base::none(),
std::chrono::seconds(2),
false
});
ws.next_layer().connect(es.stream());
ws.handshake("localhost", "/");
flat_buffer b;
bool got_timeout = false;
ws.async_read(b,
[&](error_code ec, std::size_t)
{
if(ec != beast::error::timeout)
BOOST_THROW_EXCEPTION(
system_error{ec});
got_timeout = true;
});
// We are connected, base state

test::run_for(ioc_, std::chrono::milliseconds(1250));
// After 1.25s idle, no timeout (but idle counter is 1)

es.async_ping();
test::run_for(ioc_, std::chrono::milliseconds(500));
// The server sent data at 1.25s mark, and we're now at 1.75s mark.
// We haven't hit the idle timer yet (happens at 1s, 2s, 3s)

test::run_for(ioc_, std::chrono::milliseconds(750));
// At 2.5s total; should have triggered the idle timer (but no timeout)
BEAST_EXPECT(!got_timeout);

test::run_for(ioc_, std::chrono::milliseconds(750));
// At 3s total; should have triggered the idle timer again without
// activity and triggered timeout.
BEAST_EXPECT(got_timeout);
}

// inactivity timeout doesn't happen when you send pings
{
echo_server es{log};
stream<test::stream> ws{ioc_};
ws.set_option(stream_base::timeout{
stream_base::none(),
std::chrono::seconds(1),
true
});
unsigned n_pongs = 0;
ws.control_callback({[&](frame_type kind, string_view)
{
if (kind == frame_type::pong)
++n_pongs;
}});
ws.next_layer().connect(es.stream());
ws.handshake("localhost", "/");
flat_buffer b;
ws.async_read(b, test::fail_handler(asio::error::operation_aborted));
// We are connected, base state
test::run_for(ioc_, std::chrono::seconds(2));
// About a second later, we should have close to 4 pings/pongs, and no timeout
BEAST_EXPECTS(2 <= n_pongs && n_pongs <= 4, "Unexpected nr of pings: " + std::to_string(n_pongs));
}
}

void
Expand Down
6 changes: 6 additions & 0 deletions test/beast/websocket/test.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ class websocket_test_suite
this));
}

void
async_ping()
{
ws_.async_ping("", [](error_code){});
}

void
async_close()
{
Expand Down
Loading