diff --git a/include/pistache/mailbox.h b/include/pistache/mailbox.h index 26b0d84d1..e149ba6f9 100644 --- a/include/pistache/mailbox.h +++ b/include/pistache/mailbox.h @@ -311,10 +311,47 @@ namespace Pistache if (isBound()) { - uint64_t val; + uint64_t val = 0; for (;;) { ssize_t bytes = read(event_fd, &val, sizeof val); + + // Note: Without the read-success check below, there can be + // an issue that shows up in the test + // multiple_client_with_requests_to_multithreaded_server + // when calling run_http_server_test over and over again. + // + // Without the check, in a typical success case the read in + // this function would be called twice. First time, read + // reads val. Second time, read fails with errno = EAGAIN / + // EWOULDBLOCK, causing us to break out of the loop and + // return from our "pop" function here. + // + // However, in the problem case, very occasionally two + // pushes - and hence two writes to event_fd - occur just + // ahead of the read, and both writes succeed by the time + // we are calling read the second time in the for(;;) + // loop. This causes the _second_ read to succeeed, which + // clears the eventfd readiness caused by the write, even + // though we have yet to pop that push off the queue. Hence + // the values that were pushed might never be processed off + // of this queue. So, we should break out of the loop when + // the read succeeds. + if (bytes == (sizeof val)) + { // success + + if (!ret) + { + // Have another try at pop, in case there was no + // "pop" to do at the top of this function, but + // then a push happened after the first pop attempt + // but before the read above + ret = Queue::pop(); + } + + break; + } + if (bytes == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) diff --git a/tests/http_server_test.cc b/tests/http_server_test.cc index 94b20585c..0032bd36c 100644 --- a/tests/http_server_test.cc +++ b/tests/http_server_test.cc @@ -345,6 +345,41 @@ TEST(http_server_test, multiple_client_with_requests_to_multithreaded_server) ASSERT_EQ(res2, SECOND_CLIENT_REQUEST_SIZE); } +TEST(http_server_test, many_client_with_requests_to_multithreaded_server) +{ + const Pistache::Address address("localhost", Pistache::Port(0)); + + Http::Endpoint server(address); + auto flags = Tcp::Options::ReuseAddr; + auto server_opts = Http::Endpoint::options().flags(flags).threads(6); + server.init(server_opts); + LOGGER("test", "Trying to run server..."); + server.setHandler(Http::make_handler()); + ASSERT_NO_THROW(server.serveThreaded()); + + const std::string server_address = "localhost:" + server.getPort().toString(); + LOGGER("test", "Server address: " << server_address); + + const int NO_TIMEOUT = 0; + const int SECONDS_TIMOUT = 10; + const int FIRST_CLIENT_REQUEST_SIZE = 128; + std::future result1(std::async(clientLogicFunc, + FIRST_CLIENT_REQUEST_SIZE, server_address, + NO_TIMEOUT, SECONDS_TIMOUT)); + const int SECOND_CLIENT_REQUEST_SIZE = 192; + std::future result2( + std::async(clientLogicFunc, SECOND_CLIENT_REQUEST_SIZE, server_address, + NO_TIMEOUT, SECONDS_TIMOUT)); + + int res1 = result1.get(); + int res2 = result2.get(); + + server.shutdown(); + + ASSERT_EQ(res1, FIRST_CLIENT_REQUEST_SIZE); + ASSERT_EQ(res2, SECOND_CLIENT_REQUEST_SIZE); +} + TEST(http_server_test, multiple_client_with_different_requests_to_multithreaded_server) { @@ -893,7 +928,8 @@ struct ContentEncodingHandler : public Http::Handler writer.setCompression(encoding); // Set compression level... - switch(encoding) { + switch (encoding) + { #ifdef PISTACHE_USE_CONTENT_ENCODING_BROTLI // Set maximum compression if using Brotli diff --git a/version.txt b/version.txt index 507fe4874..8dbc7056d 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -0.2.7.20240223 +0.2.7.20240405