Skip to content

Commit

Permalink
websockets: Count pings from server as activity for idle_timeout
Browse files Browse the repository at this point in the history
If the stream is receiving control packets like ping, don't count it as
idle. This means you can enable `timeout_opt.keep_alive_ping` on only
one side to get heartbeat.

Make tests that verify current behaviour.

Update documentation to match changes in PR #2718.

Addresses issue #2716
  • Loading branch information
xim committed Dec 26, 2023
1 parent d0dd9c5 commit 46f80ff
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 12 deletions.
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
73 changes: 73 additions & 0 deletions test/beast/websocket/ping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,79 @@ 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::milliseconds(2000),
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
BEAST_EXPECT(ws.impl_->idle_counter == 0);

test::run_for(ioc_, std::chrono::milliseconds(1250));
// After 1.25s idle, no timeout but idle counter is 1
BEAST_EXPECT(ws.impl_->idle_counter == 1);

es.async_ping();
test::run_for(ioc_, std::chrono::milliseconds(500));
// The server sent a ping 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)
BEAST_EXPECT(ws.impl_->idle_counter == 0);
BEAST_EXPECT(!got_timeout);

test::run_for(ioc_, std::chrono::milliseconds(750));
// At 2.5s total; should have triggered the idle timer
BEAST_EXPECT(ws.impl_->idle_counter == 1);
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::milliseconds(600),
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(1));
// About a second later, we should have close to 5 pings/pongs, and no timeout
BEAST_EXPECTS(2 <= n_pongs && n_pongs <= 3, "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

0 comments on commit 46f80ff

Please sign in to comment.