From 96f6980b5d0d3117fe7e3b69fe9daf42cdb8bb83 Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 20 Apr 2026 12:53:42 -0700 Subject: [PATCH 1/2] [WARP] Fix misc UI thread safety issues 1. Not holding mutex when clearing fetchers processed guids 2. Not managing the lifetime of the fetchers background thread correctly on current function navigation Fixes https://github.com/Vector35/binaryninja-api/issues/8111 --- plugins/warp/ui/matches.cpp | 24 +++++++++++++++--------- plugins/warp/ui/shared/fetcher.cpp | 1 + 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/plugins/warp/ui/matches.cpp b/plugins/warp/ui/matches.cpp index 918312fdd..a097380d5 100644 --- a/plugins/warp/ui/matches.cpp +++ b/plugins/warp/ui/matches.cpp @@ -8,6 +8,7 @@ #include #include #include +#include WarpCurrentFunctionWidget::WarpCurrentFunctionWidget(QWidget* parent) : QWidget(parent) { @@ -151,22 +152,27 @@ void WarpCurrentFunctionWidget::SetCurrentFunction(FunctionRef current) m_current = current; m_infoWidget->SetAnalysisFunction(m_current); - // If we have a fetcher we should also let it know to try and fetch the possible functions from the containers. + // If we have a fetcher, we should also let it know to try and fetch the possible functions from the containers. if (current && m_fetcher) { m_fetcher->AddPendingFunction(current); - - // TODO: Automatically fetch the function, I need to figure out how to make this debounce correctly so that all - // requests are processed in line. if (!m_fetcher->m_requestInProgress.exchange(true)) { - BinaryNinja::WorkerPriorityEnqueue([this]() { - QMetaObject::invokeMethod(this, [this] { m_spinner->show(); }, Qt::QueuedConnection); + // Don't block the UI thread when fetching, also unlike other cases where we reach for QtConcurrent::run we + // do not use a watcher to tie the future to the widget's lifetime, this is because network requests can + // take a long time, and we want to avoid blocking the UI thread in the widget destructor. Instead of + // insuring the widget is alive, we just use a weak pointer that can tell us when it (self) has been + // destructed. + auto future = QtConcurrent::run([self = QPointer(this), current, fetcher = m_fetcher]() { + if (!self) + return; + QMetaObject::invokeMethod(self, [self] { self->m_spinner->show(); }, Qt::QueuedConnection); BinaryNinja::Ref bgTask = new BinaryNinja::BackgroundTask("Fetching WARP Functions...", true); - const auto allowedTags = GetAllowedTagsFromView(m_current->GetView()); - m_fetcher->FetchPendingFunctions(allowedTags); + const auto allowedTags = GetAllowedTagsFromView(current->GetView()); + fetcher->FetchPendingFunctions(allowedTags); bgTask->Finish(); - QMetaObject::invokeMethod(this, [this] { m_spinner->hide(); }, Qt::QueuedConnection); + if (self) + QMetaObject::invokeMethod(self, [self] { self->m_spinner->hide(); }, Qt::QueuedConnection); }); } } diff --git a/plugins/warp/ui/shared/fetcher.cpp b/plugins/warp/ui/shared/fetcher.cpp index f5aa626a9..daa3fece5 100644 --- a/plugins/warp/ui/shared/fetcher.cpp +++ b/plugins/warp/ui/shared/fetcher.cpp @@ -113,6 +113,7 @@ void WarpFetcher::FetchPendingFunctions(const std::vector& allo void WarpFetcher::ClearProcessed() { + std::lock_guard lock(m_requestMutex); m_logger->LogInfoF("Clearing {} processed functions from cache...", m_processedGuids.size()); m_processedGuids.clear(); } From 9e849636fbf7af6498d66851d94171da2adf82ea Mon Sep 17 00:00:00 2001 From: Mason Reed Date: Mon, 20 Apr 2026 14:01:00 -0700 Subject: [PATCH 2/2] Also guard invokeMethod calls These lambdas execute on main thread so as long as it is checked before use in the lambda it is safe --- plugins/warp/ui/matches.cpp | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/plugins/warp/ui/matches.cpp b/plugins/warp/ui/matches.cpp index a097380d5..2af173834 100644 --- a/plugins/warp/ui/matches.cpp +++ b/plugins/warp/ui/matches.cpp @@ -161,18 +161,29 @@ void WarpCurrentFunctionWidget::SetCurrentFunction(FunctionRef current) // Don't block the UI thread when fetching, also unlike other cases where we reach for QtConcurrent::run we // do not use a watcher to tie the future to the widget's lifetime, this is because network requests can // take a long time, and we want to avoid blocking the UI thread in the widget destructor. Instead of - // insuring the widget is alive, we just use a weak pointer that can tell us when it (self) has been + // ensuring the widget is alive, we just use a weak pointer that can tell us when it (self) has been // destructed. auto future = QtConcurrent::run([self = QPointer(this), current, fetcher = m_fetcher]() { if (!self) return; - QMetaObject::invokeMethod(self, [self] { self->m_spinner->show(); }, Qt::QueuedConnection); + QMetaObject::invokeMethod( + self, + [self] { + if (self) + self->m_spinner->show(); + }, + Qt::QueuedConnection); BinaryNinja::Ref bgTask = new BinaryNinja::BackgroundTask("Fetching WARP Functions...", true); const auto allowedTags = GetAllowedTagsFromView(current->GetView()); fetcher->FetchPendingFunctions(allowedTags); bgTask->Finish(); - if (self) - QMetaObject::invokeMethod(self, [self] { self->m_spinner->hide(); }, Qt::QueuedConnection); + QMetaObject::invokeMethod( + self, + [self] { + if (self) + self->m_spinner->hide(); + }, + Qt::QueuedConnection); }); } }