diff --git a/ixwebsocket/IXWebSocketTransport.cpp b/ixwebsocket/IXWebSocketTransport.cpp index 6ca7d576..ac999cc6 100644 --- a/ixwebsocket/IXWebSocketTransport.cpp +++ b/ixwebsocket/IXWebSocketTransport.cpp @@ -228,6 +228,8 @@ namespace ix // No state change, return if (_readyState == readyState) return; + // Callbacks must observe the new state, including reentrant close() calls. + _readyState = readyState; if (readyState == ReadyState::CLOSED) { if (_onCloseCallback) @@ -244,8 +246,6 @@ namespace ix initTimePointsAfterConnect(); _pongReceived = false; } - - _readyState = readyState; } void WebSocketTransport::setOnCloseCallback(const OnCloseCallback& onCloseCallback) @@ -327,6 +327,7 @@ namespace ix WebSocketTransport::PollResult WebSocketTransport::poll() { + if (isAbnormalClose()) return PollResult::AbnormalClose; if (_readyState == ReadyState::OPEN) { if (pingIntervalExceeded()) @@ -399,6 +400,8 @@ namespace ix return PollResult::AbnormalClose; } + if (isAbnormalClose()) return PollResult::AbnormalClose; + if (_readyState == ReadyState::CLOSING && closingDelayExceeded()) { _rxbuf.clear(); @@ -901,7 +904,8 @@ namespace ix bool compress, const OnProgressCallback& onProgressCallback) { - if (_readyState != ReadyState::OPEN && _readyState != ReadyState::CLOSING) + if (isAbnormalClose() || + (_readyState != ReadyState::OPEN && _readyState != ReadyState::CLOSING)) { return WebSocketSendInfo(false); } @@ -1130,9 +1134,22 @@ namespace ix wsheader_type::TEXT_FRAME, message, _enablePerMessageDeflate, onProgressCallback); } + bool WebSocketTransport::isAbnormalClose() const + { + return _readyState == ReadyState::CLOSING && + _closeCode == WebSocketCloseConstants::kAbnormalCloseCode; + } + bool WebSocketTransport::sendOnSocket() { - std::lock_guard lock(_txbufMutex); + std::unique_lock lock(_txbufMutex); + if (isAbnormalClose() || _readyState == ReadyState::CLOSED) + { + _txbuf.clear(); + _txbufPending.clear(); + _txbufPendingOffset = 0; + return false; + } while (!_txbufPending.empty() || !_txbuf.empty()) { @@ -1154,14 +1171,10 @@ namespace ix } else if (ret <= 0) { - _txbuf.clear(); - _txbufPending.clear(); - _txbufPendingOffset = 0; - closeSocket(); - if (_readyState != ReadyState::CLOSING) - { - setReadyState(ReadyState::CLOSED); - } + // close() takes the send-buffer lock when discarding an aborted connection. + lock.unlock(); + close(WebSocketCloseConstants::kAbnormalCloseCode, + WebSocketCloseConstants::kAbnormalCloseMessage); return false; } else @@ -1212,6 +1225,7 @@ namespace ix else { _rxbuf.insert(_rxbuf.end(), _readbuf.begin(), _readbuf.begin() + ret); + if (isAbnormalClose()) break; } } @@ -1303,6 +1317,23 @@ namespace ix } } + // An abnormal close has no handshake. Discard pending output and let the I/O + // thread close the socket and deliver the terminal callback. Never send 1006 on wire. + if (code == WebSocketCloseConstants::kAbnormalCloseCode) + { + { + std::lock_guard lock(_txbufMutex); + _txbuf.clear(); + _txbufPending.clear(); + _txbufPendingOffset = 0; + } + if (_readyState != ReadyState::CLOSED) + { + wakeUpFromPoll(SelectInterrupt::kCloseRequest); + } + return; + } + if (!startedClose) { // Wake up the socket polling thread, as @@ -1359,8 +1390,8 @@ namespace ix if (result == PollResultType::Error) { - closeSocket(); - setReadyState(ReadyState::CLOSED); + close(WebSocketCloseConstants::kAbnormalCloseCode, + WebSocketCloseConstants::kAbnormalCloseMessage); return false; } else if (result == PollResultType::ReadyForWrite) @@ -1377,16 +1408,14 @@ namespace ix // as abnormal close and use "Send Timeout" for the reason. if (now > start + timeoutSecs) { - closeSocketAndSwitchToClosedState(WebSocketCloseConstants::kAbnormalCloseCode, - WebSocketCloseConstants::kSendTimeoutMessage, - 0, - false); + close(WebSocketCloseConstants::kAbnormalCloseCode, + WebSocketCloseConstants::kSendTimeoutMessage); return false; } } } - return true; + return _readyState != ReadyState::CLOSED && !isAbnormalClose(); } void WebSocketTransport::setCloseReason(const std::string& reason) diff --git a/ixwebsocket/IXWebSocketTransport.h b/ixwebsocket/IXWebSocketTransport.h index 24b13cc9..498159b5 100644 --- a/ixwebsocket/IXWebSocketTransport.h +++ b/ixwebsocket/IXWebSocketTransport.h @@ -273,6 +273,7 @@ namespace ix bool wakeUpFromPoll(uint64_t wakeUpCode); + bool isAbnormalClose() const; bool flushSendBuffer(); bool sendOnSocket(); bool receiveFromSocket(); diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 25690fae..63c19c73 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -32,6 +32,7 @@ set (TEST_TARGET_NAMES IXWebSocketIPv6Test IXWebSocketSendTimeoutTest IXWebSocketSendBufferTest + IXWebSocketSendFailureTest ) # Some unittest don't work on windows yet @@ -118,7 +119,7 @@ foreach(TEST_TARGET_NAME ${TEST_TARGET_NAMES}) endforeach() -set_tests_properties(IXWebSocketReconnectTest IXWebSocketDispatchTest PROPERTIES TIMEOUT 30) +set_tests_properties(IXWebSocketReconnectTest IXWebSocketDispatchTest IXWebSocketSendFailureTest PROPERTIES TIMEOUT 30) if (USE_NETWORK_FRAMEWORK OR (UNIX AND USE_TLS AND (USE_OPEN_SSL OR USE_LIBRE_SSL OR USE_MBED_TLS))) # Only the test credential generator needs the openssl command-line tool. diff --git a/test/IXWebSocketSendBufferTest.cpp b/test/IXWebSocketSendBufferTest.cpp index 36c3efdf..d101858e 100644 --- a/test/IXWebSocketSendBufferTest.cpp +++ b/test/IXWebSocketSendBufferTest.cpp @@ -88,5 +88,19 @@ TEST_CASE("Fatal sends discard both pending and newly queued bytes", "[send_buff socket.fail = true; CHECK_FALSE(ix::WebSocketTransportTestAccess::flush(transport)); CHECK(transport.bufferedAmount() == 0); + // The sender starts an abnormal close; only the I/O thread delivers the callback. + CHECK(transport.getReadyState() == ix::WebSocketTransport::ReadyState::CLOSING); + transport.dispatch( + ix::WebSocketTransport::PollResult::CannotFlushSendBuffer, + [](const std::string&, size_t, bool, ix::WebSocketTransport::MessageKind) { }); CHECK(transport.getReadyState() == ix::WebSocketTransport::ReadyState::CLOSED); + + ix::WebSocketTransportTestAccess::reset(transport); + PartialWriteSocket nextSocket; + nextSocket.writesBeforeBlocking = -1; + installWriteSocket(transport, nextSocket); + const auto fresh = writeTestMessage(41, 20); + ix::WebSocketTransportTestAccess::enqueue(transport, fresh); + flushWriteTest(transport); + CHECK(nextSocket.received == fresh); } diff --git a/test/IXWebSocketSendFailureTest.cpp b/test/IXWebSocketSendFailureTest.cpp new file mode 100644 index 00000000..77db26ed --- /dev/null +++ b/test/IXWebSocketSendFailureTest.cpp @@ -0,0 +1,222 @@ +/* + * IXWebSocketSendFailureTest.cpp + * Copyright (c) 2026 Machine Zone, Inc. All rights reserved. + */ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace ix +{ + struct WebSocketTransportTestAccess + { + static void install(WebSocketTransport& transport, + std::unique_ptr socket, + bool blocking) + { + transport._socket = std::move(socket); + transport._useMask = false; + transport._blockingSend = blocking; + transport._sendTimeoutSecs = 1; + transport._rxbuf = {0x81, 3, 'o', 'n', 'e', 0x81, 3, 't', 'w', 'o'}; + transport.setReadyState(WebSocketTransport::ReadyState::OPEN); + } + }; +} // namespace ix + +namespace +{ + using Transport = ix::WebSocketTransport; + + class FailedWriteSocket final : public ix::Socket + { + public: + explicit FailedWriteSocket(int mode) + : mode(mode) + { + } + + std::ptrdiff_t send(char*, size_t) override + { + ++writes; + setErrno(mode < 2 ? EIO : EWOULDBLOCK); + return -1; + } + + ix::PollResultType isReadyToWrite(int) override + { + if (mode == 3) + { + std::this_thread::sleep_for(std::chrono::milliseconds(1100)); + return ix::PollResultType::Timeout; + } + if (mode == 4) + { + ++polls; + std::this_thread::sleep_for(std::chrono::milliseconds(1)); + return ix::PollResultType::Timeout; + } + return ix::PollResultType::Error; + } + + void close() override + { + ++closes; + } + + const int mode; + std::atomic polls {0}; + std::atomic writes {0}; + std::atomic closes {0}; + }; +} // namespace + +TEST_CASE("Send failures finish callbacks on the I/O thread", "[websocket][close][send_failure]") +{ + // Client fatal write, server fatal write, server flush error, server send timeout. + const int mode = GENERATE(0, 1, 2, 3); + CAPTURE(mode); + Transport transport; + auto* socket = new FailedWriteSocket(mode); + ix::WebSocketTransportTestAccess::install( + transport, std::unique_ptr(socket), mode != 0); + std::mutex mutex; + std::condition_variable changed; + bool entered = false; + bool release = false; + bool messageActive = false; + bool overlapped = false; + bool closedInCallback = false; + uint16_t closeCode = 0; + std::string closeReason; + std::vector events; + std::vector callbackThreads; + transport.setOnCloseCallback( + [&](uint16_t code, const std::string& reason, size_t, bool) + { + { + std::lock_guard lock(mutex); + overlapped = messageActive; + closedInCallback = transport.getReadyState() == Transport::ReadyState::CLOSED; + closeCode = code; + closeReason = reason; + events.push_back("close"); + callbackThreads.push_back(std::this_thread::get_id()); + } + // Calling close() from a terminal notification must not deadlock. + if (closedInCallback) transport.close(); + }); + std::thread::id ioThread; + auto owner = std::async( + std::launch::async, + [&] + { + ioThread = std::this_thread::get_id(); + auto message = [&](const std::string& value, size_t, bool, Transport::MessageKind) + { + std::unique_lock lock(mutex); + events.push_back(value); + callbackThreads.push_back(std::this_thread::get_id()); + if (value == "one") + { + messageActive = true; + entered = true; + changed.notify_all(); + changed.wait(lock, [&] { return release; }); + messageActive = false; + } + }; + transport.dispatch(Transport::PollResult::Succeeded, message); + transport.dispatch(transport.poll(), message); + }); + bool received; + { + std::unique_lock lock(mutex); + received = changed.wait_for(lock, std::chrono::seconds(2), [&] { return entered; }); + } + auto sender = + std::async(std::launch::async, + [&] { return transport.sendText(std::string("outgoing"), nullptr).success; }); + const bool returned = sender.wait_for(std::chrono::seconds(3)) == std::future_status::ready; + { + std::lock_guard lock(mutex); + release = true; + } + changed.notify_all(); + const bool success = sender.get(); + owner.get(); + REQUIRE(received); + REQUIRE(returned); + CHECK_FALSE(success); + CHECK_FALSE(overlapped); + CHECK(closedInCallback); + CHECK(closeCode == ix::WebSocketCloseConstants::kAbnormalCloseCode); + CHECK(closeReason == (mode == 3 ? ix::WebSocketCloseConstants::kSendTimeoutMessage + : ix::WebSocketCloseConstants::kAbnormalCloseMessage)); + CHECK(events == std::vector {"one", "close"}); + CHECK(callbackThreads == std::vector(2, ioThread)); + CHECK(socket->closes == 1); + CHECK(socket->writes == 1); // An abnormal close must not attempt a Close frame. + CHECK(transport.bufferedAmount() == 0); +} + +TEST_CASE("Closing cancels a blocked send without waiting for its timeout", + "[websocket][close][send_failure]") +{ + Transport transport; + auto* socket = new FailedWriteSocket(4); + ix::WebSocketTransportTestAccess::install(transport, std::unique_ptr(socket), true); + auto sender = std::async(std::launch::async, + [&] { return transport.sendText(std::string("outgoing"), nullptr); }); + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + while (socket->polls == 0 && std::chrono::steady_clock::now() < deadline) + std::this_thread::yield(); + const bool blocked = socket->polls != 0; + const auto start = std::chrono::steady_clock::now(); + transport.close(4000, "cancelled"); + const bool returned = + sender.wait_for(std::chrono::milliseconds(500)) == std::future_status::ready; + sender.get(); + CHECK(blocked); + CHECK(returned); + CHECK(std::chrono::steady_clock::now() - start < std::chrono::milliseconds(500)); + CHECK(transport.getReadyState() == Transport::ReadyState::CLOSING); +} + +TEST_CASE("A close request keeps its code when a write fails", "[websocket][close][send_failure]") +{ + const bool closeFirst = GENERATE(false, true); + Transport transport; + ix::WebSocketTransportTestAccess::install( + transport, std::unique_ptr(new FailedWriteSocket(0)), false); + uint16_t code = 0; + unsigned notifications = 0; + transport.setOnCloseCallback( + [&](uint16_t value, const std::string&, size_t, bool) + { + code = value; + ++notifications; + CHECK_FALSE(transport.sendText(std::string("after close"), nullptr).success); + }); + if (!closeFirst) CHECK_FALSE(transport.sendText(std::string("outgoing"), nullptr).success); + transport.close(4000, "cancelled"); + auto message = [](const std::string&, size_t, bool, Transport::MessageKind) { }; + transport.dispatch(transport.poll(), message); + transport.dispatch(transport.poll(), message); + CHECK(code == (closeFirst ? 4000 : ix::WebSocketCloseConstants::kAbnormalCloseCode)); + CHECK(notifications == 1); +} + +TEST_CASE("Abnormal close before connecting is harmless", "[websocket][close][send_failure]") +{ + Transport transport; + transport.close(ix::WebSocketCloseConstants::kAbnormalCloseCode); + CHECK(transport.getReadyState() == Transport::ReadyState::CLOSED); +}