Update websocket idle timer on read events

boostorg · Feb 15, 2024 · 0c43571 · 0c43571
1 parent a6fd05c
commit 0c43571
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 32 deletions.
diff --git a/include/boost/beast/websocket/impl/handshake.hpp b/include/boost/beast/websocket/impl/handshake.hpp
@@ -173,7 +173,7 @@ class stream<NextLayer, deflateSupported>::handshake_op
                 goto upcall;
 
             // success
-            impl.reset_idle();
+            impl.update_timer(this->get_executor());
             impl.on_response(d_.p.get(), key_, ec);
             if(res_p_)
                 swap(d_.p.get(), *res_p_);

diff --git a/include/boost/beast/websocket/impl/read.hpp b/include/boost/beast/websocket/impl/read.hpp
@@ -197,7 +197,7 @@ class stream<NextLayer, deflateSupported>::read_some_op
                     impl.rd_buf.commit(bytes_transferred);
                     if(impl.check_stop_now(ec))
                         goto upcall;
-                    impl.reset_idle();
+                    impl.update_timer(this->get_executor());
 
                     // Allow a close operation
                     // to acquire the read block
@@ -441,7 +441,7 @@ class stream<NextLayer, deflateSupported>::read_some_op
                         impl.rd_buf.commit(bytes_transferred);
                         if(impl.check_stop_now(ec))
                             goto upcall;
-                        impl.reset_idle();
+                        impl.update_timer(this->get_executor());
                         if(impl.rd_fh.mask)
                             detail::mask_inplace(buffers_prefix(clamp(
                                 impl.rd_remain), impl.rd_buf.data()),
@@ -490,7 +490,7 @@ class stream<NextLayer, deflateSupported>::read_some_op
                         }
                         if(impl.check_stop_now(ec))
                             goto upcall;
-                        impl.reset_idle();
+                        impl.update_timer(this->get_executor());
                         BOOST_ASSERT(bytes_transferred > 0);
                         auto const mb = buffers_prefix(
                             bytes_transferred, cb_);
@@ -542,7 +542,7 @@ class stream<NextLayer, deflateSupported>::read_some_op
                         }
                         if(impl.check_stop_now(ec))
                             goto upcall;
-                        impl.reset_idle();
+                        impl.update_timer(this->get_executor());
                         BOOST_ASSERT(bytes_transferred > 0);
                         impl.rd_buf.commit(bytes_transferred);
                         if(impl.rd_fh.mask)

diff --git a/include/boost/beast/websocket/impl/stream_impl.hpp b/include/boost/beast/websocket/impl/stream_impl.hpp
@@ -439,8 +439,12 @@ struct stream<NextLayer, deflateSupported>::impl_type
             if(timeout_opt.idle_timeout != none())
             {
                 idle_counter = 0;
-                timer.expires_after(
-                    timeout_opt.idle_timeout / 2);
+                if(timeout_opt.keep_alive_pings)
+                    timer.expires_after(
+                        timeout_opt.idle_timeout / 2);
+                else
+                    timer.expires_after(
+                        timeout_opt.idle_timeout);
 
                 BOOST_ASIO_HANDLER_LOCATION((
                     __FILE__, __LINE__,
@@ -565,9 +569,9 @@ struct stream<NextLayer, deflateSupported>::impl_type
                 if(impl.timeout_opt.idle_timeout == none())
                     return;
 
-                if( impl.idle_counter < 1 )
+                if( impl.timeout_opt.keep_alive_pings &&
+                    impl.idle_counter < 1)
                 {
-                    if( impl.timeout_opt.keep_alive_pings )
                     {
                         BOOST_ASIO_HANDLER_LOCATION((
                             __FILE__, __LINE__,

diff --git a/test/beast/websocket/ping.cpp b/test/beast/websocket/ping.cpp
@@ -93,11 +93,9 @@ class ping_test : public websocket_test_suite
         {
             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),
+                std::chrono::milliseconds(1000),
                 false
             });
             ws.next_layer().connect(es.stream());
@@ -112,28 +110,11 @@ class ping_test : public websocket_test_suite
                             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);
-
+            test::run_for(ioc_, std::chrono::milliseconds(600));
             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);
+            test::run_for(ioc_, std::chrono::milliseconds(600));
             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.
+            test::run_for(ioc_, std::chrono::milliseconds(600));
             BEAST_EXPECT(got_timeout);
         }