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:

  1. Connection is opened and a GET request is issued. Turns out to be a cache miss
  2. Compilation starts and takes 90s, meanwhile the server closes the connection
  3. A PUT is issued on the now closed connection
  4. 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

Most upvoted comments

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_NOSIGNAL fix the problem on Linux on your machine?