Sfoglia il codice sorgente

Let ClientImpl::stop() interrupt a request waiting on Windows (#2653)

stop() shuts the socket down to end a request in flight, but on Windows
that does not wake a read waiting in WSAPoll (#1396). The request then
waited until the server answered, the read timeout passed, or the OS
dropped the connection.

stop() now also sets a flag, and send_() gives the request's stream an
interrupt check that reads it, as SSEClient::stop() does since 801bf2e.
The wait for data then ends within 100 ms on every platform. A request
started after stop() clears the flag. A redirect or a digest retry sent
inside the request keeps it.

select_read_interruptible() now rounds its last wait up to a whole
millisecond. poll() takes milliseconds, and the wait ended up to 1 ms
before the deadline, which MaxTimeoutTest caught once every client
request waited through this function.
Yoshiki Matsuda 1 settimana fa
parent
commit
a18895f42e
2 ha cambiato i file con 68 aggiunte e 4 eliminazioni
  1. 19 4
      httplib.h
  2. 49 0
      test/test.cc

+ 19 - 4
httplib.h

@@ -3058,6 +3058,10 @@ protected:
   bool socket_should_be_closed_when_request_is_done_ = false;
   size_t socket_tls_settings_version_ = 0;
 
+  // Set by stop() while a request is in flight. The request's reads poll it,
+  // because shutting the socket down does not wake a read on Windows.
+  std::atomic<bool> stop_requested_{false};
+
   std::atomic<size_t> tls_settings_version_{0};
 
   // Hostname to connection target map. The value is an IP literal or another
@@ -6840,10 +6844,14 @@ select_read_interruptible(socket_t sock, time_t sec, time_t usec,
   while (!interrupted()) {
     auto left = std::chrono::duration_cast<std::chrono::microseconds>(
         deadline - std::chrono::steady_clock::now());
-    auto wait =
-        (std::max)((std::min)(left, slice), std::chrono::microseconds(0));
-    auto ret = select_impl(sock, POLLIN, 0, static_cast<time_t>(wait.count()));
-    if (ret != 0 || wait < slice) { return ret; }
+    auto last = left < slice;
+    // poll() takes whole milliseconds, so round the last wait up to keep it
+    // from ending before the deadline.
+    auto wait_usec = last ? (std::max)((left.count() + 999) / 1000 * 1000,
+                                       static_cast<decltype(left.count())>(0))
+                          : slice.count();
+    auto ret = select_impl(sock, POLLIN, 0, static_cast<time_t>(wait_usec));
+    if (ret != 0 || last) { return ret; }
   }
   return 0;
 }
@@ -15803,6 +15811,9 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) {
     // socket.
     socket_should_be_closed_when_request_is_done_ = false;
 
+    // A redirect or a retry sent inside a request keeps its stop()
+    if (socket_requests_in_flight_ == 0) { stop_requested_ = false; }
+
     if (!socket_reusable()) {
       if (!ensure_socket_connection(socket_, error)) { return false; }
 
@@ -15850,6 +15861,7 @@ inline bool ClientImpl::send_(Request &req, Response &res, Error &error) {
   });
 
   ret = process_socket(socket_, req.start_time_, [&](Stream &strm) {
+    strm.set_read_interrupt([this] { return stop_requested_.load(); });
     return handle_request(strm, req, res, close_connection, error);
   });
 
@@ -18068,6 +18080,9 @@ inline void ClientImpl::stop() {
   // are not thread-safe.
   if (socket_requests_in_flight_ > 0) {
     shutdown_socket(socket_);
+    // shutdown() does not wake a read waiting in WSAPoll on Windows, so the
+    // request's reads also give up once they see this flag.
+    stop_requested_ = true;
 
     // Aside from that, we set a flag for the socket to be closed when we're
     // done.

+ 49 - 0
test/test.cc

@@ -3445,6 +3445,55 @@ TEST(CancelTest, WithCancelLargePayloadDelete) {
   EXPECT_EQ(Error::Canceled, res.error());
 }
 
+// One stop() from another thread ends a request that waits for the response
+TEST(ClientStopTest, StopEndsWaitForResponse) {
+  Server svr;
+  std::atomic<bool> received{false};
+  std::atomic<bool> done{false};
+
+  svr.Get("/wait", [&](const Request & /*req*/, Response &res) {
+    received.store(true);
+    while (!done.load()) {
+      std::this_thread::sleep_for(std::chrono::milliseconds(10));
+    }
+    res.set_content("Hello World!", "text/plain");
+  });
+  auto port = svr.bind_to_any_port(HOST);
+  auto thread = std::thread([&]() { svr.listen_after_bind(); });
+
+  Client cli(HOST, port);
+  cli.set_read_timeout(std::chrono::seconds(30));
+  std::future<Result> request;
+
+  auto se = detail::scope_exit([&] {
+    done.store(true);
+    svr.stop();
+    thread.join();
+    ASSERT_FALSE(svr.is_running());
+  });
+
+  svr.wait_until_ready();
+
+  request = std::async(std::launch::async, [&]() { return cli.Get("/wait"); });
+
+  for (int i = 0; i < 500 && !received.load(); ++i) {
+    std::this_thread::sleep_for(std::chrono::milliseconds(10));
+  }
+  ASSERT_TRUE(received.load());
+  // Let the client block in the wait for the response
+  std::this_thread::sleep_for(std::chrono::milliseconds(100));
+
+  auto start = std::chrono::steady_clock::now();
+  cli.stop();
+  ASSERT_EQ(std::future_status::ready,
+            request.wait_for(std::chrono::seconds(5)));
+  EXPECT_LT(std::chrono::steady_clock::now() - start, std::chrono::seconds(2));
+
+  auto res = request.get();
+  ASSERT_TRUE(!res);
+  EXPECT_EQ(Error::Read, res.error());
+}
+
 static std::string remove_whitespace(const std::string &input) {
   std::string output;
   output.reserve(input.size());