From 4290b90509c6b9b1cf4fa38e780f1aff5ad453e1 Mon Sep 17 00:00:00 2001 From: Glenn Smith Date: Fri, 21 Jan 2022 18:06:11 -0500 Subject: Fix race in BackgroundThread --- ui/progresstask.h | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/progresstask.h b/ui/progresstask.h index 009e2c67..88cb7609 100644 --- a/ui/progresstask.h +++ b/ui/progresstask.h @@ -496,6 +496,12 @@ public: { m_then.push_back({MainThread, [=](QVariant v) { QVariant result; + // Since the task starts immediately, we need to hold a lock to its value + // Just in case it manages to get to the part of the lambda where it reads the value + // before this thread actually assigns it. + // This is *probably* not a race in practice due to the variable being stored on the stack before construction. + std::mutex taskMutex; + taskMutex.lock(); ProgressTask* task = new ProgressTask(parent, title, text, cancel, [&](ProgressFunction progress) { auto innerProgress = [=](size_t cur, size_t max) { // Fix dialog disappearing if the backgrounded task thinks it's done @@ -507,13 +513,18 @@ public: }; try { + // See above comment about race conditions + taskMutex.lock(); + ProgressTask* innerTask = task; + taskMutex.unlock(); + if constexpr (std::is_void_v>) { - func(v, task, innerProgress); + func(v, innerTask, innerProgress); } else { - result = func(v, task, innerProgress); + result = func(v, innerTask, innerProgress); } // And actually report success progress(1, 1); @@ -524,6 +535,7 @@ public: std::rethrow_exception(std::current_exception()); }; }); + taskMutex.unlock(); task->wait(); return result; -- cgit v1.3.1