Bläddra i källkod

Report only a caller-set WebSocket read timeout as Timeout

199d7ee made read() return the new ReadResult::Timeout for every read
timeout and leave the connection open. The compile-time server default
(CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND, 300s) is always in
effect, so a handler written as `while (ws.read(msg))`, the form the
README's Quick Start uses, no longer ended when a peer went quiet:
Timeout is non-zero, so the loop ran its body again with the previous
message still in `msg`, and the worker the backstop is meant to reclaim
was never released. Nothing caught it because every test of the new
result set a timeout explicitly and checked the result by value, and the
heartbeat tests keep the connection alive with pings.

The two timeouts mean different things. One the caller sets through
set_read_timeout() is a request for control back, and is reported as
Timeout on a still-open connection. The compile-time default is a
backstop against a peer that has gone quiet, and elapsing it is now a
failure again: read() returns Fail and closes the connection, as it did
before 199d7ee. WebSocket tracks whether set_read_timeout() was called,
and WebSocketClient carries the same flag over to the WebSocket it
creates on connect().

Tests use the heartbeat binary, which compiles both defaults down to 3s:
a `while (ws.read(msg))` server handler runs its body once and exits
when the client falls silent, and a client that never set a timeout gets
Fail with the connection closed. The README and cookbook now say which
timeout produces Timeout.

Claude-Session: https://claude.ai/code/session_01EF5uZ1X2kaHhqJ8VgfjVaQ
yhirose 3 veckor sedan
förälder
incheckning
6b8c3f5387

+ 2 - 2
README-websocket.md

@@ -66,9 +66,9 @@ enum ReadResult : int {
 
 Returned by `read()`. Since `Fail` is `0`, the result works naturally in boolean contexts — `while (ws.read(msg))` continues until the connection closes. When you need to distinguish text from binary, check the return value directly.
 
-`Timeout` only appears once a read timeout is in effect (a client waits forever unless you set one; a server uses `CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND`). It means the timeout elapsed on a message boundary: nothing was consumed and the connection is still open, so you can send on it and read again.
+`Timeout` is only returned for a read timeout you set yourself with `set_read_timeout()`. It means the timeout elapsed on a message boundary: nothing was consumed and the connection is still open, so you can send on it and read again. The compile-time defaults (`CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND`, 300 seconds on the server; a client waits forever) are a backstop against a peer that has gone quiet, not a request for control: when one of them elapses, `read()` returns `Fail` and closes the connection, so code that never calls `set_read_timeout()` can keep using `while (ws.read(msg))`.
 
-**`msg` is left untouched on `Timeout`.** Because `Timeout` is non-zero, `while (ws.read(msg))` keeps looping — with the *previous* message still in `msg`. Once a read timeout is set, test the result instead:
+**`msg` is left untouched on `Timeout`.** Because `Timeout` is non-zero, `while (ws.read(msg))` keeps looping — with the *previous* message still in `msg`. Once you set a read timeout, test the result instead:
 
 ```cpp
 ws.set_read_timeout(std::chrono::milliseconds(100));

+ 1 - 1
docs-src/pages/en/cookbook/w01-websocket-echo.md

@@ -36,7 +36,7 @@ The `read()` return value is a `ReadResult` enum:
 - `ReadResult::Text`: received a text message
 - `ReadResult::Binary`: received a binary message
 - `ReadResult::Fail`: error, or connection closed
-- `ReadResult::Timeout`: the read timeout elapsed with nothing received; the connection is still open. Only appears once a read timeout is set — see [W06. Set Timeouts](../w06-websocket-timeouts)
+- `ReadResult::Timeout`: a read timeout you set with `set_read_timeout()` elapsed with nothing received; the connection is still open. The compile-time default timeout closes the connection and is reported as `Fail` instead — see [W06. Set Timeouts](../w06-websocket-timeouts)
 
 ## Client: talk to the echo server
 

+ 2 - 0
docs-src/pages/en/cookbook/w06-websocket-timeouts.md

@@ -77,6 +77,8 @@ A handler's `ws::WebSocket` has `set_read_timeout()` too, and the pattern above
 
 The server default is 300s (`CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND`) rather than "forever": it is a backstop that reclaims a worker from a peer that has gone silent, since a WebSocket handler holds its worker for the life of the connection.
 
+Because it is a backstop rather than something the handler asked for, it does not surface as `Timeout`. When it elapses, `read()` returns `Fail` and closes the connection, so a handler written as `while (ws.read(msg))` ends the way it always has. Only a timeout the handler set itself with `set_read_timeout()` comes back as `Timeout`.
+
 > Unresponsive-peer detection via Ping/Pong is a separate mechanism. See [W02. Set a WebSocket Heartbeat](../w02-websocket-ping) for details.
 
 ## How this differs from `Client`

+ 1 - 1
docs-src/pages/ja/cookbook/w01-websocket-echo.md

@@ -36,7 +36,7 @@ int main() {
 - `ReadResult::Text`: テキストメッセージを受信
 - `ReadResult::Binary`: バイナリメッセージを受信
 - `ReadResult::Fail`: エラー、または接続が閉じた
-- `ReadResult::Timeout`: 何も受信しないまま読み取りタイムアウトが経過した。接続は開いたまま。読み取りタイムアウトを設定したときだけ返る([W06. タイムアウトを設定する](../w06-websocket-timeouts)を参照)
+- `ReadResult::Timeout`: `set_read_timeout()`で自分が設定した読み取りタイムアウトが、何も受信しないまま経過した。接続は開いたまま。コンパイル時のデフォルトのタイムアウトは接続を閉じ、`Fail`として返る([W06. タイムアウトを設定する](../w06-websocket-timeouts)を参照)
 
 ## クライアント: エコーを叩く
 

+ 2 - 0
docs-src/pages/ja/cookbook/w06-websocket-timeouts.md

@@ -77,6 +77,8 @@ while (ws.is_open()) {
 
 サーバ側のデフォルトは「無期限」ではなく300秒(`CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND`)です。WebSocketのハンドラは接続が続く限りワーカーを1つ占有するので、無言になったピアからワーカーを回収する保険として働きます。
 
+この保険はハンドラが求めたタイムアウトではないので、`Timeout`としては返りません。経過すると`read()`は`Fail`を返して接続を閉じるため、`while (ws.read(msg))`と書いたハンドラは従来どおりそこで終わります。`Timeout`が返るのは、ハンドラ自身が`set_read_timeout()`で設定したタイムアウトだけです。
+
 > Ping/Pongによる無応答ピア検出は別の仕組みです。詳しくは[W02. ハートビートを設定する](../w02-websocket-ping)を参照してください。
 
 ## `Client`との違い

+ 24 - 3
httplib.h

@@ -235,7 +235,10 @@
 #endif
 
 // 0 waits forever. A read timeout is how a caller gets control back to send on
-// the same connection; it is not a liveness check (that is ping/pong).
+// the same connection; it is not a liveness check (that is ping/pong). Only a
+// timeout set at runtime through set_read_timeout() is reported as
+// ws::Timeout; when one of these compile-time defaults elapses, read() returns
+// ws::Fail and closes the connection.
 #ifndef CPPHTTPLIB_WEBSOCKET_CLIENT_READ_TIMEOUT_SECOND
 #define CPPHTTPLIB_WEBSOCKET_CLIENT_READ_TIMEOUT_SECOND 0
 #endif
@@ -4443,6 +4446,11 @@ public:
   // Bound how long read() waits before returning Timeout. 0 waits forever.
   // A server handler owns its connection's timeout this way; a client sets it
   // through WebSocketClient. Safe to call while another thread is in read().
+  //
+  // Only a timeout set here is reported as Timeout. The compile-time default
+  // (CPPHTTPLIB_WEBSOCKET_SERVER_READ_TIMEOUT_SECOND) is a backstop rather
+  // than a request for control, so when it elapses read() returns Fail and
+  // closes the connection, and `while (ws.read(msg))` ends as it always has.
   void set_read_timeout(time_t sec, time_t usec = 0);
   template <class Rep, class Period>
   void set_read_timeout(const std::chrono::duration<Rep, Period> &duration);
@@ -4482,6 +4490,10 @@ private:
   int max_missed_pongs_;
   int unacked_pings_ = 0;
   std::atomic<bool> closed_{false};
+  // Set once the caller has bounded read() through set_read_timeout(). Until
+  // then the timeout in effect is the compile-time default, and elapsing it
+  // is a failure that closes the connection, not a Timeout.
+  std::atomic<bool> read_timeout_set_{false};
   std::mutex write_mutex_;
   // Owned by whichever thread is parsing frames off strm_. Only one thread
   // may do so: read_websocket_frame() reads a payload until it has the whole
@@ -4571,6 +4583,7 @@ private:
   std::unique_ptr<WebSocket> ws_;
   time_t read_timeout_sec_ = CPPHTTPLIB_WEBSOCKET_CLIENT_READ_TIMEOUT_SECOND;
   time_t read_timeout_usec_ = 0;
+  bool read_timeout_set_ = false; // see WebSocket::read_timeout_set_
   time_t write_timeout_sec_ = CPPHTTPLIB_CLIENT_WRITE_TIMEOUT_SECOND;
   time_t write_timeout_usec_ = CPPHTTPLIB_CLIENT_WRITE_TIMEOUT_USECOND;
   time_t websocket_ping_interval_sec_ =
@@ -22311,8 +22324,11 @@ inline ReadResult WebSocket::read(std::string &msg) {
         impl::read_websocket_frame(strm_, opcode, payload, fin, is_server_,
                                    CPPHTTPLIB_WEBSOCKET_MAX_PAYLOAD_LENGTH);
     // A timeout landed on a frame boundary: the connection is untouched and
-    // still usable, so hand control back without closing it.
-    if (r == impl::FrameRead::Timeout) { return Timeout; }
+    // still usable, so hand control back without closing it. That is only
+    // useful to a caller who asked for the timeout; the compile-time default
+    // is a backstop against a peer gone quiet, and elapsing it closes the
+    // connection so a plain `while (ws.read(msg))` loop ends.
+    if (r == impl::FrameRead::Timeout && read_timeout_set_) { return Timeout; }
     if (r != impl::FrameRead::Ok) {
       closed_ = true;
       return Fail;
@@ -22503,6 +22519,7 @@ inline void WebSocket::set_read_timeout(time_t sec, time_t usec) {
   // negative poll uses for an unbounded wait.
   if (sec == 0 && usec == 0) { sec = -1; }
   strm_.set_read_timeout(sec, usec);
+  read_timeout_set_ = true;
 }
 
 // WebSocketClient implementation
@@ -22727,6 +22744,9 @@ inline Result WebSocketClient::connect() {
   ws_ = std::unique_ptr<WebSocket>(new WebSocket(std::move(strm), req, false,
                                                  websocket_ping_interval_sec_,
                                                  websocket_max_missed_pongs_));
+  // The stream was created with the timeout already; tell the WebSocket
+  // whether it came from the caller, so read() knows to report it as Timeout.
+  ws_->read_timeout_set_ = read_timeout_set_;
   return Result{Error::Success, upgrade.status, std::move(upgrade.headers)};
 }
 
@@ -22759,6 +22779,7 @@ inline const std::string &WebSocketClient::subprotocol() const {
 inline void WebSocketClient::set_read_timeout(time_t sec, time_t usec) {
   read_timeout_sec_ = sec;
   read_timeout_usec_ = usec;
+  read_timeout_set_ = true;
   // The members above only seed the next connect(); read() consults the
   // stream, so an already-open connection has to be told directly.
   if (ws_) { ws_->set_read_timeout(sec, usec); }

+ 72 - 0
test/test_websocket_heartbeat.cc

@@ -9,6 +9,8 @@
 
 #include "gtest/gtest.h"
 
+#include <future>
+
 using namespace httplib;
 
 class WebSocketHeartbeatTest : public ::testing::Test {
@@ -192,6 +194,76 @@ TEST_F(WebSocketPongTimeoutTest, ClientDetectsNonResponsivePeer) {
   EXPECT_FALSE(client.is_open());
 }
 
+// The compile-time client read timeout (3s here) was never asked for through
+// set_read_timeout(), so when it elapses read() reports Fail and closes the
+// connection rather than handing back a Timeout on a still-open one.
+TEST_F(WebSocketPongTimeoutTest, CompileTimeClientReadTimeoutIsFail) {
+  ws::WebSocketClient client("ws://localhost:" + std::to_string(port_) + "/ws");
+  client.set_websocket_ping_interval(0);
+  ASSERT_TRUE(client.connect());
+
+  // Server pings are off and its handler never sends, so nothing arrives.
+  std::string msg;
+  EXPECT_EQ(client.read(msg), ws::Fail);
+  EXPECT_FALSE(client.is_open());
+}
+
+// The compile-time server read timeout (3s here) is a backstop that reclaims
+// the worker from a peer gone quiet, not a timeout the handler asked for. When
+// it elapses read() must return Fail, so a handler written as
+// `while (ws.read(msg))` ends instead of re-running its body with the previous
+// message still in `msg`.
+class WebSocketServerReadTimeoutTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    svr_.set_websocket_ping_interval(0);
+    svr_.WebSocket("/ws", [this](const Request &, ws::WebSocket &ws) {
+      std::string msg;
+      while (ws.read(msg)) {
+        iterations_++;
+        ws.send(msg);
+      }
+      handler_done_.set_value();
+    });
+
+    port_ = svr_.bind_to_any_port("localhost");
+    thread_ = std::thread([this]() { svr_.listen_after_bind(); });
+    svr_.wait_until_ready();
+  }
+
+  void TearDown() override {
+    svr_.stop();
+    thread_.join();
+  }
+
+  Server svr_;
+  int port_;
+  std::thread thread_;
+  std::atomic<int> iterations_{0};
+  std::promise<void> handler_done_;
+};
+
+TEST_F(WebSocketServerReadTimeoutTest, BackstopEndsHandlerLoop) {
+  ws::WebSocketClient client("ws://localhost:" + std::to_string(port_) + "/ws");
+  client.set_websocket_ping_interval(0); // nothing reaches the server's read()
+  client.set_read_timeout(10, 0);        // fail rather than hang
+  ASSERT_TRUE(client.connect());
+
+  ASSERT_TRUE(client.send("hello"));
+  std::string msg;
+  ASSERT_EQ(client.read(msg), ws::Text);
+  EXPECT_EQ("hello", msg);
+
+  // The client now stays silent. The server's backstop elapses and the
+  // handler returns, having run its loop body exactly once.
+  auto done = handler_done_.get_future();
+  ASSERT_EQ(done.wait_for(std::chrono::seconds(6)), std::future_status::ready);
+  EXPECT_EQ(1, iterations_.load());
+
+  EXPECT_EQ(client.read(msg), ws::Fail);
+  EXPECT_FALSE(client.is_open());
+}
+
 // Verify that a responsive peer does NOT trigger the pong-timeout mechanism,
 // even with a small max_missed_pongs budget. This is the positive counterpart
 // of ClientDetectsNonResponsivePeer: the client must actively drive read() so