From fc2b6b63dbeb2047783a5c83c70b89df82e44369 Mon Sep 17 00:00:00 2001 From: Max Liberman Date: Thu, 5 Mar 2026 16:13:26 -0500 Subject: [PATCH] Fix pure virtual call crash in BackgroundTransportCurl destructor In the async path, SignalTransferComplete() was called before CompleteOperation(). This unblocked the waiting thread (via WaitForAllTransfersToComplete), allowing it to destroy the Request and Response objects. When CompleteOperation() then called request_->MarkCompleted() / response_->MarkFailed(), these were dangling pointers whose vtables had been overwritten during destruction, causing _purecall -> abort(). The fix is to always call CompleteOperation() before SignalTransferComplete(), which is what the synchronous path already did. This ensures request_/response_ are still alive when their virtual methods are called. The old comment worried that MarkCompleted/MarkFailed "could end up attempting to tear down TransportCurl", but these methods only set a boolean flag and cannot trigger destruction. Even if the complete_ callback somehow triggered TransportCurl teardown, ~TransportCurl calls WaitForAllTransfersToComplete() which would block until SignalTransferComplete() is called, so the ordering is self-protecting. This crash manifests as BROWSER-WIN-2SRG in Sentry with 19k+ events: EXCEPTION_ACCESS_VIOLATION_WRITE via _purecall in BackgroundTransportCurl::~BackgroundTransportCurl. --- app/rest/transport_curl.cc | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/app/rest/transport_curl.cc b/app/rest/transport_curl.cc index 46712b0dc2..6b3aca38a7 100644 --- a/app/rest/transport_curl.cc +++ b/app/rest/transport_curl.cc @@ -394,19 +394,12 @@ BackgroundTransportCurl::~BackgroundTransportCurl() { request_header_ = nullptr; } - // If this is an asynchronous operation, MarkFailed() or MarkCompleted() - // could end up attempting to tear down TransportCurl so we signal - // completion here. - if (transport_curl_->is_async()) { - transport_curl_->SignalTransferComplete(); - CompleteOperation(); - } else { - // Synchronous operations need all data present in the response before - // Perform() returns so signal complete after MarkFailed() or - // MarkCompleted(). - CompleteOperation(); - transport_curl_->SignalTransferComplete(); - } + // Complete the operation before signaling, so that request_/response_ are + // still valid when MarkCompleted()/MarkFailed() is called. Signaling first + // would allow the waiting thread to destroy the Request/Response objects + // before CompleteOperation() can use them, causing a pure virtual call crash. + CompleteOperation(); + transport_curl_->SignalTransferComplete(); } void BackgroundTransportCurl::CompleteOperation() {