Loading src/httpd.cpp +21 −4 Original line number Diff line number Diff line Loading @@ -589,13 +589,30 @@ void libhttppp::HttpEvent::_dispatchH1Request(HttpRequest &cureq, size_t consume std::shared_ptr<netplus::con> connOwner = fd >= 0 ? netplus::lookupConnection(fd) : nullptr; std::unique_ptr<netplus::socket> sock = connOwner ? netplus::detachConnection(cureq) : nullptr; if (connOwner && sock) { cureq.slots[0].csock = std::move(sock); cureq.slots[0].csock->setBlock(); cureq.slots[0].csock->setTimeout(kH1OffloadSocketTimeoutMs); // Deliberately leave cureq.slots[0].csock null here -- detachConnection()'s normal // post-condition. IoEventHandler has an existing check immediately after its // RequestEvent(con&,...) call site that sees a null csock and returns right away // without touching the connection further (the same mechanism the WebSocket-upgrade // handoff in proxyplus relies on). Restoring csock here instead would let that // still-running caller's own post-dispatch work (flushing SendData, EpollArmGuard's // destructor re-arming the fd for EPOLLIN) keep touching the same socket // concurrently with the task below -- a real data race, not a hypothetical one; it's // exactly what a previous version of this function did and it deadlocked/crashed // under load. The socket is boxed in a shared_ptr purely so a move-only unique_ptr // can live inside a std::function (ThreadPool::submit's parameter type), same as // _dispatchH2Stream's trBox below. auto sockBox = std::make_shared<std::unique_ptr<netplus::socket>>(std::move(sock)); _h1DispatchPool->submit( [this, connOwner, consumeBodyBytes, tid, args]() mutable { [this, connOwner, sockBox, consumeBodyBytes, tid, args]() mutable { HttpRequest &cureq2 = static_cast<HttpRequest&>(*connOwner); // Safe without event_mutex: nothing else can observe this connection while // it's detached from CONNECTIONS (no epoll/kqueue worker will ever dispatch // to this fd again until reattachConnection() below puts it back). cureq2.slots[0].csock = std::move(*sockBox); cureq2.slots[0].csock->setBlock(); cureq2.slots[0].csock->setTimeout(kH1OffloadSocketTimeoutMs); try { // The blocking work this whole detach/reattach dance exists to keep // off the shared epoll/kqueue workers. Loading test/httpd_h1_offload_test.cpp +42 −29 Original line number Diff line number Diff line Loading @@ -136,10 +136,13 @@ std::string doKeepAliveGet(netplus::tcp &sock, const std::string &path) { for (;;) { size_t headerEnd = buf.find("\r\n\r\n"); if (headerEnd != std::string::npos) { size_t clPos = buf.find("Content-Length:"); // HttpResponse::printHeader() writes header keys verbatim as stored // (lowercase -- e.g. setHeaderData("content-length") -- not // capitalized the way a browser-facing example might show it). size_t clPos = buf.find("content-length:"); if (clPos == std::string::npos || clPos > headerEnd) throw std::runtime_error("doKeepAliveGet: no Content-Length in response"); size_t clStart = clPos + std::strlen("Content-Length:"); throw std::runtime_error("doKeepAliveGet: no content-length in response"); size_t clStart = clPos + std::strlen("content-length:"); size_t clEnd = buf.find("\r\n", clStart); size_t contentLength = std::stoul(buf.substr(clStart, clEnd - clStart)); size_t bodyStart = headerEnd + 4; Loading Loading @@ -194,10 +197,16 @@ int main() { const unsigned workerCount = (std::max)(1u, std::thread::hardware_concurrency()); const int slowDelayMs = 300; // Returns -1 on any failure (connection/response trouble) instead of throwing -- // startServer/stopServer's process-wide statics (see RunningServer's own comment) // mean the server must always be cleanly stopped before this function returns, // which a mid-function throw would skip. auto runSaturationCase = [&](size_t h1OffloadThreads) -> long long { int port = 0; RunningServer rs = startServer(port, h1OffloadThreads); long long result = -1; try { std::vector<std::unique_ptr<netplus::tcp>> slowSocks(workerCount); for (unsigned i = 0; i < workerCount; ++i) { slowSocks[i] = std::make_unique<netplus::tcp>(); Loading @@ -218,13 +227,17 @@ int main() { std::string body = doKeepAliveGet(fastSock, "/fast"); auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::steady_clock::now() - t0).count(); if (body != "ok") throw std::runtime_error("fast request got wrong body"); fastSock.close(); for (auto &s : slowSocks) s->close(); if (body == "ok") result = elapsedMs; else std::cerr << "FAIL: fast request got wrong body ('" << body << "')" << std::endl; } catch (std::exception &e) { std::cerr << "FAIL: saturation case threw: " << e.what() << std::endl; } stopServer(rs); return elapsedMs; return result; }; long long syncMs = runSaturationCase(/*h1OffloadThreads=*/0); Loading @@ -243,7 +256,7 @@ int main() { std::cout << "[offload] fast request latency while " << workerCount << " slow requests saturate every worker: " << offloadMs << "ms " "(expect < ~" << slowDelayMs / 2 << "ms, unaffected)" << std::endl; if (offloadMs >= slowDelayMs / 2) { if (offloadMs < 0 || offloadMs >= slowDelayMs / 2) { std::cerr << "FAIL: offloaded slow requests still starved the fast one " "(latency=" << offloadMs << "ms)" << std::endl; ++failures; Loading Loading
src/httpd.cpp +21 −4 Original line number Diff line number Diff line Loading @@ -589,13 +589,30 @@ void libhttppp::HttpEvent::_dispatchH1Request(HttpRequest &cureq, size_t consume std::shared_ptr<netplus::con> connOwner = fd >= 0 ? netplus::lookupConnection(fd) : nullptr; std::unique_ptr<netplus::socket> sock = connOwner ? netplus::detachConnection(cureq) : nullptr; if (connOwner && sock) { cureq.slots[0].csock = std::move(sock); cureq.slots[0].csock->setBlock(); cureq.slots[0].csock->setTimeout(kH1OffloadSocketTimeoutMs); // Deliberately leave cureq.slots[0].csock null here -- detachConnection()'s normal // post-condition. IoEventHandler has an existing check immediately after its // RequestEvent(con&,...) call site that sees a null csock and returns right away // without touching the connection further (the same mechanism the WebSocket-upgrade // handoff in proxyplus relies on). Restoring csock here instead would let that // still-running caller's own post-dispatch work (flushing SendData, EpollArmGuard's // destructor re-arming the fd for EPOLLIN) keep touching the same socket // concurrently with the task below -- a real data race, not a hypothetical one; it's // exactly what a previous version of this function did and it deadlocked/crashed // under load. The socket is boxed in a shared_ptr purely so a move-only unique_ptr // can live inside a std::function (ThreadPool::submit's parameter type), same as // _dispatchH2Stream's trBox below. auto sockBox = std::make_shared<std::unique_ptr<netplus::socket>>(std::move(sock)); _h1DispatchPool->submit( [this, connOwner, consumeBodyBytes, tid, args]() mutable { [this, connOwner, sockBox, consumeBodyBytes, tid, args]() mutable { HttpRequest &cureq2 = static_cast<HttpRequest&>(*connOwner); // Safe without event_mutex: nothing else can observe this connection while // it's detached from CONNECTIONS (no epoll/kqueue worker will ever dispatch // to this fd again until reattachConnection() below puts it back). cureq2.slots[0].csock = std::move(*sockBox); cureq2.slots[0].csock->setBlock(); cureq2.slots[0].csock->setTimeout(kH1OffloadSocketTimeoutMs); try { // The blocking work this whole detach/reattach dance exists to keep // off the shared epoll/kqueue workers. Loading
test/httpd_h1_offload_test.cpp +42 −29 Original line number Diff line number Diff line Loading @@ -136,10 +136,13 @@ std::string doKeepAliveGet(netplus::tcp &sock, const std::string &path) { for (;;) { size_t headerEnd = buf.find("\r\n\r\n"); if (headerEnd != std::string::npos) { size_t clPos = buf.find("Content-Length:"); // HttpResponse::printHeader() writes header keys verbatim as stored // (lowercase -- e.g. setHeaderData("content-length") -- not // capitalized the way a browser-facing example might show it). size_t clPos = buf.find("content-length:"); if (clPos == std::string::npos || clPos > headerEnd) throw std::runtime_error("doKeepAliveGet: no Content-Length in response"); size_t clStart = clPos + std::strlen("Content-Length:"); throw std::runtime_error("doKeepAliveGet: no content-length in response"); size_t clStart = clPos + std::strlen("content-length:"); size_t clEnd = buf.find("\r\n", clStart); size_t contentLength = std::stoul(buf.substr(clStart, clEnd - clStart)); size_t bodyStart = headerEnd + 4; Loading Loading @@ -194,10 +197,16 @@ int main() { const unsigned workerCount = (std::max)(1u, std::thread::hardware_concurrency()); const int slowDelayMs = 300; // Returns -1 on any failure (connection/response trouble) instead of throwing -- // startServer/stopServer's process-wide statics (see RunningServer's own comment) // mean the server must always be cleanly stopped before this function returns, // which a mid-function throw would skip. auto runSaturationCase = [&](size_t h1OffloadThreads) -> long long { int port = 0; RunningServer rs = startServer(port, h1OffloadThreads); long long result = -1; try { std::vector<std::unique_ptr<netplus::tcp>> slowSocks(workerCount); for (unsigned i = 0; i < workerCount; ++i) { slowSocks[i] = std::make_unique<netplus::tcp>(); Loading @@ -218,13 +227,17 @@ int main() { std::string body = doKeepAliveGet(fastSock, "/fast"); auto elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>( std::chrono::steady_clock::now() - t0).count(); if (body != "ok") throw std::runtime_error("fast request got wrong body"); fastSock.close(); for (auto &s : slowSocks) s->close(); if (body == "ok") result = elapsedMs; else std::cerr << "FAIL: fast request got wrong body ('" << body << "')" << std::endl; } catch (std::exception &e) { std::cerr << "FAIL: saturation case threw: " << e.what() << std::endl; } stopServer(rs); return elapsedMs; return result; }; long long syncMs = runSaturationCase(/*h1OffloadThreads=*/0); Loading @@ -243,7 +256,7 @@ int main() { std::cout << "[offload] fast request latency while " << workerCount << " slow requests saturate every worker: " << offloadMs << "ms " "(expect < ~" << slowDelayMs / 2 << "ms, unaffected)" << std::endl; if (offloadMs >= slowDelayMs / 2) { if (offloadMs < 0 || offloadMs >= slowDelayMs / 2) { std::cerr << "FAIL: offloaded slow requests still starved the fast one " "(latency=" << offloadMs << "ms)" << std::endl; ++failures; Loading