Skip to content

Commit 1e12bd8

Browse files
committed
[BUG] Keep an aborted session alive until its handle is freed
Handing the easy handle over untouched leaves CURLOPT_PRIVATE and the write, header, read and progress callback data pointing at the session and its operation. Clearing them was what made that safe before, and this branch stopped clearing them on purpose, because the cancelling thread does not own a handle the multi handle may still be driving. The owner then has to outlive the handle, and on the abort path it did not. ScheduleAbortSession takes the session out of sessions_, so doRemoveSessions cannot find it to hold one, and doAbortSessions kept the only remaining shared_ptr in a local map that died on return. The handle could reach curl_multi_remove_handle and curl_easy_cleanup with those pointers dangling, and curl_easy_cleanup is documented as able to run the progress and header callbacks. pending_to_remove_sessions_ already exists for this: doRemoveSessions swaps it out and holds it until the handle is freed. The abort path hands the session to it now. 26 tests pass, 3 of 3, and under AddressSanitizer with leak detection 2 of 2 with no leaks and no errors. Signed-off-by: thc1006 <84045975+thc1006@users.noreply.github.com>
1 parent 7f82d74 commit 1e12bd8

1 file changed

Lines changed: 10 additions & 1 deletion

File tree

‎ext/src/http/client/curl/http_client_curl.cc‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -765,7 +765,7 @@ bool HttpClient::doAbortSessions()
765765
}
766766

767767
bool has_data = false;
768-
for (const auto &session : pending_to_abort_sessions)
768+
for (auto &session : pending_to_abort_sessions)
769769
{
770770
if (!session.second)
771771
{
@@ -777,6 +777,15 @@ bool HttpClient::doAbortSessions()
777777
session.second->FinishOperation();
778778
has_data = true;
779779
}
780+
781+
// FinishOperation queued the easy handle for removal, and that handle still holds
782+
// CURLOPT_PRIVATE and the callback data pointing into this session and its operation.
783+
// doRemoveSessions cannot find the session to hold, because aborting took it out of
784+
// sessions_, so hand it the owner directly: it keeps one until the handle is freed.
785+
{
786+
std::lock_guard<std::recursive_mutex> session_id_lock_guard{session_ids_m_};
787+
pending_to_remove_sessions_.emplace_back(std::move(session.second));
788+
}
780789
}
781790
return has_data;
782791
}

0 commit comments

Comments
 (0)