[TEST] Let a cancelled request end either of the two ways it can

RepeatedCallerThreadCancelsAreClean summed Cancelled and Response over twenty
attempts against a closed port and asked for at least one each. There is a
third way such an attempt ends: the connection fails before the cancel arrives,
and the cancel is then correctly a no-op. Where the caller and the IO thread
share a core that is the usual outcome, and the case fails.

Measured with the run pinned to one core, fifteen runs each: 15 of 15 fail on
main at 3fb1d31, on open-telemetry#4395 at 66ab56c, and on this branch either side of the
change it carries, all with the same 3 to 7 cancels out of 20. Unpinned all
four pass. The failure the valgrind job hit is that, not anything about the
handle accounting.

The connection failure is counted apart from terminal_count_, which two other
cases pin to an exact value, and it is taken before the existing chain rather
than inside it, because ACancelBeforeTheResponseReportsCancelled cancels from
ConnectFailed and needs that branch to keep firing. The case now asks each
attempt to end somewhere the handler is told about rather than to end the same
way twenty times, which is what it was after. Pinned to one core it goes from
15 of 15 failing to 0 of 15.

Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>

Author: thc1006

ext/test/http/curl_http_test.cc

@@ -143,6 +143,14 @@ class TerminalCountingHandler : public CustomEventHandler
 
   void OnEvent(http_client::SessionState state, nostd::string_view reason) noexcept override
   {
+    // Counted on its own, and before the chain below rather than inside it: a case can cancel
+    // from one of these, and two others pin terminal_count_ to an exact value.
+    if (state == http_client::SessionState::ConnectFailed ||
+        state == http_client::SessionState::SendFailed)
+    {
+      failed_count_.fetch_add(1, std::memory_order_release);
+    }
+
     if (state == http_client::SessionState::Cancelled)
     {
       terminal_count_.fetch_add(1, std::memory_order_release);
@@ -168,6 +176,7 @@ class TerminalCountingHandler : public CustomEventHandler
   http_client::SessionState cancel_at_ = http_client::SessionState::Response;
   std::thread::id cancelled_from_{};
   std::atomic<int> terminal_count_{0};
+  std::atomic<int> failed_count_{0};
   std::atomic<int> cancelled_from_callback_{0};
 };
 
@@ -822,8 +831,6 @@ TEST_F(BasicCurlHttpTests, ACancelFromTheCallerThreadReportsCancelled)
 // thread sanitizer this reports against the unfixed client on every run.
 TEST_F(BasicCurlHttpTests, RepeatedCallerThreadCancelsAreClean)
 {
-  int terminal_total = 0;
-
   for (int i = 0; i < 20; ++i)
   {
     auto session_manager = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
@@ -840,12 +847,15 @@ TEST_F(BasicCurlHttpTests, RepeatedCallerThreadCancelsAreClean)
     session_manager->FinishAllSessions();
 
     EXPECT_FALSE(handler->got_response_.load(std::memory_order_acquire));
-    terminal_total += handler->terminal_count_.load(std::memory_order_acquire);
-  }
 
-  // A lower bound, not a count: #4360 tracks the same cancel arriving twice, and how many
-  // arrive is not what this case decides.
-  EXPECT_GE(terminal_total, 20);
+    // Whichever side wins, the attempt ends somewhere the handler is told about: the cancel
+    // lands first, or the connection to a closed port fails first and the cancel is then
+    // correctly a no-op. How many cancels arrive is not what this case decides, #4360 tracks
+    // that, and neither is which of the two wins.
+    EXPECT_GT(handler->terminal_count_.load(std::memory_order_acquire) +
+                  handler->failed_count_.load(std::memory_order_acquire),
+              0);
+  }
 }
 
 TEST_F(BasicCurlHttpTests, SendGetRequestSync)