From 155b152637b843fac06c8c71300b6e2da793ff5c Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 7 Dec 2023 15:23:28 +0100 Subject: [PATCH 1/2] Fix crash in gcc on Linux --- src/slic3r/GUI/Jobs/BoostThreadWorker.cpp | 9 ++++++--- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 2 +- src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 7 ++++++- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp index 2c3158e4cd..c390d4e3e2 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.cpp @@ -81,9 +81,12 @@ std::future BoostThreadWorker::call_on_main_thread(std::function } BoostThreadWorker::BoostThreadWorker(std::shared_ptr pri, - boost::thread::attributes &attribs, - const char * name) - : m_progress(std::move(pri)), m_input_queue{m_running}, m_output_queue{m_running}, m_name{name} + boost::thread::attributes &attribs, + const char *name) + : m_progress(std::move(pri)) + , m_input_queue{m_running} + , m_output_queue{m_running} + , m_name{name} { if (m_progress) m_progress->set_cancel_callback([this](){ cancel(); }); diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index 823ad9adfe..a8d38cf771 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -69,7 +69,7 @@ class BoostThreadWorker : public Worker, private Job::Ctl template class RawQueue: public std::deque { - std::atomic *m_running_ptr = nullptr; + std::atomic *m_running_ptr; public: using std::deque::deque; diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp index df1779c06f..05c14f9cdc 100644 --- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -21,6 +21,10 @@ struct BlockingWait unsigned timeout_ms = 0; }; +template +using NonSpecialMembersOnly = std::enable_if_t< + (sizeof...(Args) >= 1) && !(... || std::is_convertible_v)>; + // A thread safe queue for one producer and one consumer. template class Container = std::deque, @@ -32,10 +36,11 @@ class ThreadSafeQueueSPSC std::condition_variable m_cond_var; public: - template + template> ThreadSafeQueueSPSC(Qargs &&...qargs) : m_queue{Container{std::forward(qargs)...}} {} + ThreadSafeQueueSPSC() = default; ThreadSafeQueueSPSC(const ThreadSafeQueueSPSC&) = default; ThreadSafeQueueSPSC(ThreadSafeQueueSPSC&&) = default; ThreadSafeQueueSPSC& operator=(const ThreadSafeQueueSPSC&) = default; From 7975dfab26e7cc0d1d5c0070e369a4f0777131b7 Mon Sep 17 00:00:00 2001 From: tamasmeszaros Date: Thu, 7 Dec 2023 15:42:17 +0100 Subject: [PATCH 2/2] Add comments, remove some redundant template fiddling Also make ThreadSafeQueueSPSC not copyable and non movable --- src/slic3r/GUI/Jobs/BoostThreadWorker.hpp | 7 +++++++ src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp | 17 +++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp index a8d38cf771..3d11097e2a 100644 --- a/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp +++ b/src/slic3r/GUI/Jobs/BoostThreadWorker.hpp @@ -67,6 +67,11 @@ class BoostThreadWorker : public Worker, private Job::Ctl void deliver(BoostThreadWorker &runner); }; + // The m_running state flag needs special attention. Previously, it was set simply in the run() + // method whenever a new job was taken from the input queue and unset after the finalize message + // was pushed into the output queue. This was not correct. It must not be possible to consume + // the finalize message before the flag gets unset, these two operations must be done atomically + // So the underlying queues are here extended to support handling of this m_running flag. template class RawQueue: public std::deque { std::atomic *m_running_ptr; @@ -79,6 +84,7 @@ class BoostThreadWorker : public Worker, private Job::Ctl void set_stopped() { m_running_ptr->store(false); } }; + // The running flag is set if a job is popped from the queue template class RawJobQueue: public RawQueue { public: @@ -90,6 +96,7 @@ class BoostThreadWorker : public Worker, private Job::Ctl } }; + // The running flag is unset when the finalize message is pushed into the queue template class RawMsgQueue: public RawQueue { public: diff --git a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp index 05c14f9cdc..e0d423d4b9 100644 --- a/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp +++ b/src/slic3r/GUI/Jobs/ThreadSafeQueue.hpp @@ -21,10 +21,6 @@ struct BlockingWait unsigned timeout_ms = 0; }; -template -using NonSpecialMembersOnly = std::enable_if_t< - (sizeof...(Args) >= 1) && !(... || std::is_convertible_v)>; - // A thread safe queue for one producer and one consumer. template class Container = std::deque, @@ -36,15 +32,16 @@ class ThreadSafeQueueSPSC std::condition_variable m_cond_var; public: - template> + + // Forward arguments to the underlying queue + template ThreadSafeQueueSPSC(Qargs &&...qargs) : m_queue{Container{std::forward(qargs)...}} {} - ThreadSafeQueueSPSC() = default; - ThreadSafeQueueSPSC(const ThreadSafeQueueSPSC&) = default; - ThreadSafeQueueSPSC(ThreadSafeQueueSPSC&&) = default; - ThreadSafeQueueSPSC& operator=(const ThreadSafeQueueSPSC&) = default; - ThreadSafeQueueSPSC& operator=(ThreadSafeQueueSPSC &&) = default; + ThreadSafeQueueSPSC(const ThreadSafeQueueSPSC&) = delete; + ThreadSafeQueueSPSC(ThreadSafeQueueSPSC&&) = delete; + ThreadSafeQueueSPSC& operator=(const ThreadSafeQueueSPSC&) = delete; + ThreadSafeQueueSPSC& operator=(ThreadSafeQueueSPSC &&) = delete; // Consume one element, block if the queue is empty. template