diff --git a/README-websocket.md b/README-websocket.md index e52265e..657fd32 100644 --- a/README-websocket.md +++ b/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)); diff --git a/docs-src/pages/en/cookbook/w01-websocket-echo.md b/docs-src/pages/en/cookbook/w01-websocket-echo.md index ffe96d6..7fcb7b6 100644 --- a/docs-src/pages/en/cookbook/w01-websocket-echo.md +++ b/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 diff --git a/docs-src/pages/en/cookbook/w06-websocket-timeouts.md b/docs-src/pages/en/cookbook/w06-websocket-timeouts.md index 1f7742b..c8e4f4c 100644 --- a/docs-src/pages/en/cookbook/w06-websocket-timeouts.md +++ b/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` diff --git a/docs-src/pages/ja/cookbook/w01-websocket-echo.md b/docs-src/pages/ja/cookbook/w01-websocket-echo.md index 3067a8a..9e51fbc 100644 --- a/docs-src/pages/ja/cookbook/w01-websocket-echo.md +++ b/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)を参照） ## クライアント: エコーを叩く diff --git a/docs-src/pages/ja/cookbook/w06-websocket-timeouts.md b/docs-src/pages/ja/cookbook/w06-websocket-timeouts.md index ccbb1d5..4db3411 100644 --- a/docs-src/pages/ja/cookbook/w06-websocket-timeouts.md +++ b/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`との違い diff --git a/httplib.h b/httplib.h index 8604436..ff59c9c 100644 --- a/httplib.h +++ b/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 void set_read_timeout(const std::chrono::duration &duration); @@ -4482,6 +4490,10 @@ private: int max_missed_pongs_; int unacked_pings_ = 0; std::atomic 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 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 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(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); } diff --git a/test/test_websocket_heartbeat.cc b/test/test_websocket_heartbeat.cc index 389f70b..334ff10 100644 --- a/test/test_websocket_heartbeat.cc +++ b/test/test_websocket_heartbeat.cc @@ -9,6 +9,8 @@ #include "gtest/gtest.h" +#include + 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 iterations_{0}; + std::promise 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