[BUG] Finish an unregistered operation before telling its handler

The branch added for a session the client never registered reported the
failure and then finished the operation. The handler is replaceable, and
one that calls FinishSession() from that event waits on a promise that
only the Cleanup() below the dispatch can fulfil, on the same stack.

Measured: the case hung for the whole 60 second bound, twice, exit 124.
With the order reversed it returns in under a second, 3 of 3.

The terminal state goes in before Cleanup() so it still reports one
event rather than a manual cancel, and the report now happens after the
operation is done, which is the order the exporters already follow:
retire, then call anything replaceable.

The case stays, because the failure mode is a hang. Nothing else in the
file would notice the order being put back.

32 tests pass, 3 of 3.

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

Author: thc1006

ext/src/http/client/curl/http_operation_curl.cc

@@ -1489,11 +1489,16 @@ CURLcode HttpOperation::SendAsync(Session *session, std::function<void(HttpOpera
   }
   else if (!session->GetHttpClient().ScheduleAddSession(session->GetSessionId()))
   {
-    // The same, except nobody cancelled anything. Name the failure first: Cleanup() would report
-    // a manual cancel, with a reason read from a curl result that is still CURLE_OK.
+    // The same, except nobody cancelled anything. The terminal state goes in first so Cleanup()
+    // does not report a manual cancel, with a reason read from a curl result that is still
+    // CURLE_OK, and the operation is finished before the handler hears about it: a handler that
+    // calls FinishSession() from this event would otherwise wait on the promise that the
+    // Cleanup() below it is the only thing able to fulfil.
+    session_state_.store(opentelemetry::ext::http::client::SessionState::CreateFailed,
+                         std::memory_order_release);
+    Cleanup();
     DispatchEvent(opentelemetry::ext::http::client::SessionState::CreateFailed,
                   "the session is not registered with this client");
-    Cleanup();
   }
 
   return CURLE_OK;

ext/test/http/curl_http_test.cc

@@ -783,6 +783,56 @@ TEST_F(BasicCurlHttpTests, UnparsableUrlsReleaseTheirOwnResources)
   EXPECT_EQ(1, second_handler->terminal_count_.load(std::memory_order_acquire));
 }
 
+namespace
+{
+// The event is dispatched after the operation is finished, so a handler is free to call
+// FinishSession() from it. Reporting first instead leaves this waiting on a promise that only the
+// Cleanup() below the dispatch can fulfil, which hangs rather than fails, so this case has to run.
+class FinishFromEventHandler : public TerminalCountingHandler
+{
+public:
+  void OnEvent(http_client::SessionState state, nostd::string_view reason) noexcept override
+  {
+    TerminalCountingHandler::OnEvent(state, reason);
+    if (state == http_client::SessionState::CreateFailed && finish_target_ != nullptr)
+    {
+      auto *target   = finish_target_;
+      finish_target_ = nullptr;
+      entered_.store(true, std::memory_order_release);
+      target->FinishSession();
+      returned_.store(true, std::memory_order_release);
+    }
+  }
+
+  http_client::Session *finish_target_ = nullptr;
+  std::atomic<bool> entered_{false};
+  std::atomic<bool> returned_{false};
+};
+}  // namespace
+
+TEST_F(BasicCurlHttpTests, FinishFromTheCreateFailedEventReturns)
+{
+  auto session_manager = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();
+  ASSERT_TRUE(session_manager != nullptr);
+
+  auto session = session_manager->CreateSession("http://127.0.0.1:not-a-port");
+  auto request = session->CreateRequest();
+  request->SetUri("get/");
+
+  auto handler            = std::make_shared<FinishFromEventHandler>();
+  handler->finish_target_ = session.get();
+
+  session->SendRequest(handler);
+
+  ASSERT_TRUE(handler->entered_.load(std::memory_order_acquire))
+      << "the failure never reached the handler, so nothing was tested";
+  EXPECT_TRUE(handler->returned_.load(std::memory_order_acquire))
+      << "FinishSession from this event waited on a promise only its own caller can set";
+
+  session->FinishSession();
+  session_manager->FinishAllSessions();
+}
+
 TEST_F(BasicCurlHttpTests, InvalidUrlCompletes)
 {
   auto session_manager = std::make_shared<http_client::curl::HttpCurlClientFactory>()->Create();