cpp-httplib: Client keep-alive support is broken
Hello,
I recently added this http library to server HTTP requests within the ccache compiler cache. Because a compilation requires up four HTTP requests I enabled keep-alive support in the Client.
But we observed a race condition on Linux:
- Connection is opened and a GET request is issued. Turns out to be a cache miss
- Compilation starts and takes 90s, meanwhile the server closes the connection
- A PUT is issued on the now closed connection
- The ccache process receives SIGPIPE.
Somehow the is_alive detection does not work as expected. A rough test case looks like this:
TEST(KeepAlive, SimpleInterface_Online) {
const auto host = "127.0.0.1";
const auto port = 8080;
const auto resourcePath = "/hi";
Server svr;
svr.set_keep_alive_timeout(3);
svr.Get(resourcePath, [](const httplib::Request &, httplib::Response &res) {
res.set_content("Hello World!", "text/plain");
});
auto a2 = std::async(std::launch::async, [&svr, host, port]{ svr.listen(host, port); });
std::this_thread::sleep_for(std::chrono::milliseconds(200));
Client cli(host, port);
cli.set_keep_alive(true);
auto result = cli.Get(resourcePath);
ASSERT_TRUE(result);
EXPECT_EQ(200, result->status);
std::this_thread::sleep_for(std::chrono::seconds(5));
result = cli.Get(resourcePath);
ASSERT_TRUE(result);
EXPECT_EQ(200, result->status);
svr.stop();
a2.wait();
}
Besides the non-working is_alive detection the library should set the MSG_NOSIGNAL on Linux and SO_NOSIGPIPE on Apple platforms to not badly interfere with the library host process.
Thanks, Gregor
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Reactions: 2
- Comments: 27 (27 by maintainers)
Commits related to this issue
- Fix #1041 (WIP) — committed to yhirose/cpp-httplib by yhirose 3 years ago
- Fix #1041 — committed to yhirose/cpp-httplib by yhirose 3 years ago
- Fix #1041 — committed to yhirose/cpp-httplib by yhirose 3 years ago
- Fix #1041 (#1132) * Fix #1041 * Fixed problem with is_socket_alive * Adjust the way to check if the sockt is still alive. * Revert "Adjust the way to check if the sockt is still alive." T... — committed to ExclusiveOrange/cpp-httplib-exor by yhirose 3 years ago
- Fix #1041 (#1132) * Fix #1041 * Fixed problem with is_socket_alive * Adjust the way to check if the sockt is still alive. * Revert "Adjust the way to check if the sockt is still alive." T... — committed to MCV-Messenger/cpp-httplib by yhirose 3 years ago
Sorry for the delayed response… the new code works well for me also. Thanks!
@tsilia, thanks for all your reviews and suggestions for the pull request! It has finally been resolved. 😃
@gjasny, @lightray22, I would like to let you know that all the unit tests and @tsilia’s test (https://github.com/tsilia/poc-httplib-inf-keepalive) passed successfully, and merged the pull request to the master. Please let me know if you find anything.
Thanks.
@lightray22, thanks for the detailed information! I now got two evidences. I am too busy to work on it though, hope I’ll get back to this problem sometime this year.
@gjasny, for clarification, does
MSG_NOSIGNALfix the problem on Linux on your machine?