Repository navigation
[BUG] HttpOperation::Abort writes to the curl easy handle from the cancelling thread #4375
Description
Splitting this out of #4369, which is about the test that happened to expose it. dbarker asked for a separate issue to track the client side and the discussion of how to fix it.
What happens
HttpOperation::Abort() writes to the curl easy handle from whichever thread cancels:
void HttpOperation::Abort()
{
is_aborted_.store(true, std::memory_order_release);
if (curl_resource_.easy_handle != nullptr)
{
// Enable progress callback to abort from polling thread
curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_NOPROGRESS, 0L);
...The background thread is driving that same handle through the multi handle at the time. libcurl's rule is that an easy handle must not be used from two threads at once, and TSAN saw exactly that pairing:
main thread Session::CancelSession -> HttpOperation::Abort -> curl_easy_setopt
thread T17 curl_multi_perform -> Curl_connect -> resolve_server -> Curl_resolv
Full log in #4369.
Reach
Session::CancelSession() is public, and HttpClient::CancelAllSessions() calls it for every session, which is what OtlpHttpClient::Shutdown() does. So an application that shuts down while a request is in flight is in this, not only a test. It is intermittent: the window is however long the IO thread spends inside the handle, so the odds go up when the cancel lands early, during name resolution or connect.
A possible fix
OnProgressCallback reads nothing but is_aborted_, which is atomic and is already set on the line above, and the callback is registered at setup through CURLOPT_XFERINFOFUNCTION and CURLOPT_XFERINFODATA. CURLOPT_NOPROGRESS is the only piece still left at its default, which is why Abort() has to reach for the handle at all. Turning it on at setup would leave Abort() with the flag and the scheduled teardown:
void HttpOperation::Abort()
{
is_aborted_.store(true, std::memory_order_release);
if (async_data_ && nullptr != async_data_->session)
{
async_data_->session->GetHttpClient().ScheduleAbortSession(
async_data_->session->GetSessionId());
}
}The curl_resource_.easy_handle read goes too, since reading that member from the cancelling thread is the same kind of sharing.
The cost is a progress callback that runs during every transfer and does one atomic load. I have this built: 22 of 22 in curl_http_test, 19.1 s against 18.1 s on main so no slowdown, clean under OTELCPP_MAINTAINER_MODE=ON, and a probe confirms the callback really does fire, 459 times across the suite, so CURLOPT_NOPROGRESS = 0 at setup is taking effect.
I have not sent it, because it is a behaviour change in a shared client and worth agreeing on first. Happy to open it if this shape looks right.
What I could not do
Reproduce it. Twelve TSAN runs on the box, including one pointed at a slow-resolving host to widen the window, all clean. The evidence is the CI log and the code path, so whatever fix lands wants checking against the Bazel TSAN config rather than a local one.
Activity
owent commentedon Aug 9, 2026
curl_multi_wakeup()is only available since curl 7.68. InScheduleAbortSession, we use it to wake up the curl IO thread so thatcurl_multi_remove_handle()can be called to cancel the in-flight request promptly.For curl versions before 7.68,
curl_easy_setopt(curl_resource_.easy_handle, CURLOPT_NOPROGRESS, 0L)is an alternative way to stop the data transfer as soon as possible — otherwise the thread may block for a long time waiting forcurl_multi_perform(...)to return.Looking at curl's source code:
This write causes TSAN to report a data race. However, it should be safe in practice, because we have already ensured that the easy handle is still valid at this point. Do you have any suggestions on how to deal with this issue?
thc1006 commentedon Aug 10, 2026
Short answer: set
CURLOPT_NOPROGRESSinSendAsync(), besideCURLOPT_PRIVATE, rather than inAbort(), and the cancelling thread stops touching the handle at all. #4392 does that. It goes there rather than inSetup()becauseSetup()serves the synchronous path too, and nothing there can be cancelled.It keeps the pre-7.68 lever rather than putting it behind a version guard, which was my first instinct until I read your note. The wakeup only gets the IO thread out of
curl_multi_poll; the callback covers the case where it's stuck insidecurl_multi_perform. Those aren't the same case, andthird_party_minimumsaying 7.81 doesn't make the second one go away.Two things I didn't expect came out of building it.
The
setoptwasn't the race I could reproduce. A single caller-thread cancel gave nothing across twenty TSAN runs onmain. Looping the cancel twenty times per run fired on five runs out of five, and not where I was looking:async_data_->sessionis a plainSession *, read byAbort()and cleared byCleanup()on the IO thread. It's atomic in the PR. Nothing worse is hiding under it:Abort()only ever runs fromSession::CancelSession(), so the pointer is always the session whose own method is executing, and callers hold that byshared_ptr.Arming the callback exposes its return value.
OnProgressCallback()returnsCURL_PROGRESSFUNC_CONTINUEwhen nothing has been aborted, and that branch is dead today, becauseNOPROGRESSonly reaches 0 afteris_aborted_is already up and the callback returns early. Arm the option up front and it becomes the normal path.CURL_PROGRESSFUNC_CONTINUEdoesn't mean carry on quietly, it asks libcurl to run its built-in progress meter as well, and that meter writes to stderr.With the option armed and nothing differing but the return value,
curl_http_testwrites 10,305 bytes of curl progress bars to stderr against 114 bytes. Moving the option on its own would have been a bad trade. It needsreturn 0alongside it.On safe in practice, I'd rather not have to decide. With the option set at setup, nothing writes to the handle off the IO thread any more, so whether that one write happened to be harmless stops being a question anyone has to answer later.
Three other things turned up on the way, filed separately to keep this one small: #4389, the IO thread deadlocking on
sessions_m_while recovering from a multi error, reproduced with a forced branch and a core; #4390, a cancel from aConnectingevent getting erased soFinishSession()never returns, reproduced through the public API alone; #4391, the handle being reset and its header list freed before the removal from the multi handle.One more I didn't file.
AsyncData::callback_threadis a plainstd::thread::id, written on the IO thread inCleanup()and read by whoever callsFinish(), withOPENTELEMETRY_SANITIZER_NO_THREADon both accessors and a pair of standalone fences that don't have an atomic object to pair with. A torn read in the direction that compares equal would skip the future wait. Say the word and I'll write it up.