[BUG] The curl client writes HttpOperation::session_state_ from two threads without synchronization

Describe your environment main at f1e1c9cb, Linux, gcc 14.2, libcurl 8.14.1, CMake build with -fsanitize=thread. The race is in the source rather than in a build option, so the Bazel --config=tsan toolchain sees the same thing.

Steps to reproduce

HttpOperation::DispatchEvent stores the new state after the handler has returned, at http_operation_curl.cc:415:

void HttpOperation::DispatchEvent(SessionState type, const std::string &reason)
{
  if (event_handle_ != nullptr)
  {
    event_handle_->OnEvent(type, reason);
  }

  session_state_ = type;
}

session_state_ is a plain SessionState member (http_operation_curl.h:341, and SessionState is a std::uint8_t enum). Two threads reach that store for one operation whenever a handler cancels from an early event on a client that is already polling:

1. Send a request on a fresh client and let it finish. MaybeSpawnBackgroundThread() runs after SendAsync returns, so this first request is what puts an IO thread there at all.
2. Create a second session and call CancelSession() from its Connecting event, without leaving the handler yet.
3. The cancel wakes the IO thread, which runs doAbortSessions (http_client_curl.cc:777) into Session::FinishOperation (:269) into HttpOperation::Cleanup. Cleanup finds a non terminal state and dispatches a Cancelled of its own at http_operation_curl.cc:536, which stores session_state_ on the IO thread.
4. The handler returns and DispatchEvent stores session_state_ = Connecting on the caller thread.

I held the handler open for 400 ms so the overlap is wide enough to be deterministic rather than something I got lucky on. Three TSAN runs out of three report it, and five out of five reach the overlap on the behavioral build.

What is the expected behavior?

Either one thread owns the store, or the store is synchronized. And a state the operation has already left should not replace a terminal one.

What is the actual behavior?

TSAN reports a data race on the store. Trimmed to the two stacks:

WARNING: ThreadSanitizer: data race
  Write of size 1 by main thread:
    #0 HttpOperation::DispatchEvent(...) http_operation_curl.cc:415
    #1 HttpOperation::SendAsync(...) http_operation_curl.cc:1455
    #2 Session::SendRequest(...) http_client_curl.cc:209

  Previous write of size 1 by thread T2:
    #0 HttpOperation::DispatchEvent(...) http_operation_curl.cc:415
    #1 HttpOperation::Cleanup() http_operation_curl.cc:536
    #2 Session::FinishOperation() http_client_curl.cc:269
    #3 HttpClient::doAbortSessions() http_client_curl.cc:777

Past the undefined behavior, the later store wins, so GetSessionState() ends at Connecting for an operation that was cancelled. Two places read that member and act on it: Cleanup at :529 decides from it whether to dispatch at all, and the completion callback in Session::SendRequest compares it against Response to decide whether to deliver a response.

Additional context

Nothing in ext/test/http/curl_http_test.cc reaches this today, which is why Bazel tsan config is green. Every case that cancels from an early event uses a freshly created client, and there the IO thread does not exist while the event runs, so there is only one writer. That is measured rather than assumed: the same probe without the first request reports one thread inside the handler, and with it, two.

A warning for whoever writes the regression test. My first attempt counted concurrent handler entries with acq_rel atomics and TSAN went quiet. The IO thread's release RMW and the caller's later RMW on the same atomic synchronize with each other, which orders the two stores and hides the race the counters exist to catch. They have to be relaxed. I checked that with a deliberate unsynchronized counter on the same line as a control: the acq_rel build reported neither, the relaxed build reported both.

On main this scenario reports two races and then hangs. #4395 removes the hang and the other one, a write to async_data_->session in SendAsync racing the read of it in Cleanup, and leaves this one. So this is not something #4395 introduces, and it is not something #4395 fixes either.

Two shapes for a fix, and the choice does not look like mine to make:

• Make session_state_ a std::atomic. Removes the undefined behavior, leaves the later writer winning, so a cancelled operation can still report Connecting. It also changes the layout of a type in an installed header.
• Store before dispatching and refuse to leave a terminal state. That fixes the reported value too, but it changes what a handler sees when it asks GetSessionState() from inside its own event, which today is the state before the one it is being told about.

Scheduling note, same as on #4360. #4392 and #4395 are open against these two files. Tell me which shape you want and I will fit it around whatever has landed rather than sending a change that collides with them. It also holds up the regression test for the polling client half of #4390, because that test turns Bazel tsan config red on this race.

Start with HttpOperation::DispatchEvent in http_operation_curl.cc:415, the session_state_ declaration in http_operation_curl.h:341, and the Cleanup and SendRequest call paths described in the issue. Run the polling-client scenario under the Bazel tsan config and inspect ext/test/http/curl_http_test.cc. Done means the race is gone and a cancelled operation no longer ends in Connecting, with behavior for handler-visible state decided first.