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 #2980

Merged
merged 2 commits into from
Feb 9, 2025
Merged
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 .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -196,13 +196,13 @@ jobs:
- toolset: clang
install: clang-16
compiler: clang++-16
cxxstd: "11,14,17,20,2b"
cxxstd: "11,14,17,20" # no 2b: https://github.com/llvm/llvm-project/issues/97842
os: ubuntu-24.04
supported: true
- toolset: clang
install: clang-17
compiler: clang++-17
cxxstd: "11,14,17,20,2b"
cxxstd: "11,14,17,20" # no 2b: https://github.com/llvm/llvm-project/issues/97842
os: ubuntu-24.04
supported: true
- toolset: clang
Expand Down
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
2 changes: 2 additions & 0 deletions include/boost/beast/websocket/impl/read.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ class stream<NextLayer, deflateSupported>::read_some_op
// Handle ping frame
if(impl.rd_fh.op == detail::opcode::ping)
{
impl.update_timer(this->get_executor());

if(impl.ctrl_cb)
{
if(! cont)
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
69 changes: 69 additions & 0 deletions test/beast/websocket/ping.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,75 @@ class ping_test : public websocket_test_suite
{
doTestPing(AsyncClient{yield});
});

// inactivity timeout doesn't happen when you get pings
{
net::io_context ioc;
stream<test::stream> ws1(ioc);
stream<test::stream> ws2(ioc);
test::connect(ws1.next_layer(), ws2.next_layer());
ws1.set_option(stream_base::timeout{
stream_base::none(),
std::chrono::milliseconds(500),
false
});
ws2.async_accept(test::success_handler());
ws1.async_handshake("localhost", "/", test::success_handler());
ioc.run();
ioc.restart();
bool got_timeout = false;
flat_buffer b1;
ws1.async_read(b1,
[&](error_code ec, std::size_t)
{
if(ec != beast::error::timeout)
BOOST_THROW_EXCEPTION(
system_error{ec});
got_timeout = true;
});
ioc.run_for(std::chrono::milliseconds(250));
ioc.restart();
ws2.async_ping("", test::success_handler());
ioc.run_for(std::chrono::milliseconds(300));
ioc.restart();
BEAST_EXPECT(!got_timeout);
ioc.run_for(std::chrono::milliseconds(500));
ioc.restart();
BEAST_EXPECT(got_timeout);
}

// inactivity timeout doesn't happen when you send pings
{
net::io_context ioc;
stream<test::stream> ws1(ioc);
stream<test::stream> ws2(ioc);
test::connect(ws1.next_layer(), ws2.next_layer());
ws1.set_option(stream_base::timeout{
stream_base::none(),
std::chrono::milliseconds(1500),
true
});
unsigned n_pongs = 0;
ws1.control_callback({[&](frame_type kind, string_view)
{
if (kind == frame_type::pong)
++n_pongs;
}});
ws2.async_accept(test::success_handler());
ws1.async_handshake("localhost", "/", test::success_handler());
ioc.run();
ioc.restart();
flat_buffer b1, b2;
ws1.async_read(b1, test::fail_handler(asio::error::operation_aborted));
ws2.async_read(b2, test::fail_handler(error::closed));
ioc.run_for(std::chrono::seconds(2));
ioc.restart();
ws1.async_close({}, test::success_handler());
ioc.run();
ioc.restart();
// We should have close to 2 pings/pongs, and no timeout
BEAST_EXPECTS(1 <= n_pongs && n_pongs <= 3, "Unexpected nr of pings: " + std::to_string(n_pongs));
}
}

void
Expand Down
Loading