From ab83dcf30521d60e4c723a6f5c9a053b67493f03 Mon Sep 17 00:00:00 2001 From: remi durand Date: Thu, 29 Apr 2021 14:57:52 +0200 Subject: [PATCH] Fix deadlock when canceling the slicing while creating thumbnails --- src/libslic3r/Config.hpp | 2 +- src/libslic3r/GCode.cpp | 8 +++++- src/libslic3r/GCode/ThumbnailData.hpp | 2 +- src/slic3r/GUI/GLCanvas3D.cpp | 9 ++++--- src/slic3r/GUI/GLCanvas3D.hpp | 2 +- src/slic3r/GUI/Plater.cpp | 39 +++++++++++++++++++++++---- src/slic3r/GUI/Selection.cpp | 4 +-- src/slic3r/GUI/Selection.hpp | 2 +- 8 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/libslic3r/Config.hpp b/src/libslic3r/Config.hpp index a4c4ee04e..a102aaae5 100644 --- a/src/libslic3r/Config.hpp +++ b/src/libslic3r/Config.hpp @@ -169,7 +169,7 @@ enum PrinterTechnology : uint8_t // Laser engraving ptLaser = 1 << 4, // Any technology, useful for parameters compatible with both ptFFF and ptSLA - ptAny = 1+2+4, + ptAny = ptFFF | ptSLA | ptSLS | ptMill | ptLaser, // Unknown, useful for command line processing ptUnknown = 1 << 7 }; diff --git a/src/libslic3r/GCode.cpp b/src/libslic3r/GCode.cpp index 7a7a14e97..2fb86e58b 100644 --- a/src/libslic3r/GCode.cpp +++ b/src/libslic3r/GCode.cpp @@ -849,10 +849,16 @@ namespace DoExport { for (const Vec2d &size : sizes) if (size.x() > 0 && size.y() > 0) good_sizes.push_back(size); + if (good_sizes.empty()) return; + //Create the thumbnails const size_t max_row_length = 78; ThumbnailsList thumbnails; - thumbnail_cb(thumbnails, good_sizes, true, true, thumbnails_with_bed, true); + // note that it needs the gui thread, so can create deadlock if job is canceled. + bool can_create_thumbnail = thumbnail_cb(thumbnails, good_sizes, true, true, thumbnails_with_bed, true); + throw_if_canceled(); + if (!can_create_thumbnail) return; + for (const ThumbnailData& data : thumbnails) { if (data.is_valid()) diff --git a/src/libslic3r/GCode/ThumbnailData.hpp b/src/libslic3r/GCode/ThumbnailData.hpp index 2a302ed85..b5232c660 100644 --- a/src/libslic3r/GCode/ThumbnailData.hpp +++ b/src/libslic3r/GCode/ThumbnailData.hpp @@ -20,7 +20,7 @@ struct ThumbnailData }; typedef std::vector ThumbnailsList; -typedef std::function ThumbnailsGeneratorCallback; +typedef std::function ThumbnailsGeneratorCallback; } // namespace Slic3r diff --git a/src/slic3r/GUI/GLCanvas3D.cpp b/src/slic3r/GUI/GLCanvas3D.cpp index b4040ee86..f5fc53b27 100644 --- a/src/slic3r/GUI/GLCanvas3D.cpp +++ b/src/slic3r/GUI/GLCanvas3D.cpp @@ -1206,7 +1206,7 @@ GLCanvas3D::GLCanvas3D(wxGLCanvas* canvas) GLCanvas3D::~GLCanvas3D() { - reset_volumes(); + reset_volumes(true); } void GLCanvas3D::post_event(wxEvent &&event) @@ -1303,7 +1303,7 @@ unsigned int GLCanvas3D::get_volumes_count() const return (unsigned int)m_volumes.volumes.size(); } -void GLCanvas3D::reset_volumes() +void GLCanvas3D::reset_volumes(bool is_destroying) { if (!m_initialized) return; @@ -1313,11 +1313,12 @@ void GLCanvas3D::reset_volumes() _set_current(); - m_selection.clear(); + m_selection.clear(is_destroying); m_volumes.clear(); m_dirty = true; - _set_warning_texture(WarningTexture::ObjectOutside, false); + if(!is_destroying) + _set_warning_texture(WarningTexture::ObjectOutside, false); } int GLCanvas3D::check_volumes_outside_state() const diff --git a/src/slic3r/GUI/GLCanvas3D.hpp b/src/slic3r/GUI/GLCanvas3D.hpp index d26a6c459..1d3f4ac40 100644 --- a/src/slic3r/GUI/GLCanvas3D.hpp +++ b/src/slic3r/GUI/GLCanvas3D.hpp @@ -558,7 +558,7 @@ public: unsigned int get_volumes_count() const; const GLVolumeCollection& get_volumes() const { return m_volumes; } - void reset_volumes(); + void reset_volumes(bool is_destroying = false); int check_volumes_outside_state() const; void reset_gcode_toolpaths() { m_gcode_viewer.reset(); } diff --git a/src/slic3r/GUI/Plater.cpp b/src/slic3r/GUI/Plater.cpp index 3b28c1339..740d9d505 100644 --- a/src/slic3r/GUI/Plater.cpp +++ b/src/slic3r/GUI/Plater.cpp @@ -2001,14 +2001,43 @@ Plater::priv::priv(Plater *q, MainFrame *main_frame) background_process.set_fff_print(&fff_print); background_process.set_sla_print(&sla_print); background_process.set_gcode_result(&gcode_result); - background_process.set_thumbnail_cb([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background) + background_process.set_thumbnail_cb([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background)->bool { - std::packaged_task task([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background) { + auto task = std::make_shared>([this](ThumbnailsList& thumbnails, const Vec2ds& sizes, bool printable_only, bool parts_only, bool show_bed, bool transparent_background) { generate_thumbnails(thumbnails, sizes, printable_only, parts_only, show_bed, transparent_background); }); - std::future result = task.get_future(); - wxTheApp->CallAfter([&]() { task(thumbnails, sizes, printable_only, parts_only, show_bed, transparent_background); }); - result.wait(); + + std::future future_result = task->get_future(); + std::shared_ptr protect_bool = std::make_shared(); + std::shared_ptr is_started = std::make_shared(false); + std::shared_ptr cancel = std::make_shared(false); + wxTheApp->CallAfter([task, protect_bool, is_started, cancel, &thumbnails, &sizes, &printable_only, &parts_only, &show_bed, &transparent_background]() + { + { + std::lock_guard lock(*protect_bool); + if (*cancel) + return; + *is_started = true; + } + (*task)(thumbnails, sizes, printable_only, parts_only, show_bed, transparent_background); + }); + // can deadlock if background processing is cancelled / locked + // have to cancel the process if we're exiting here as the parameters will be deleted. + // if the process is already started, then we have to wait its end. and there is no deadlock with generate_thumbnails + // 2 seconds is plenty to + std::future_status result = future_result.wait_for(std::chrono::seconds(2)); + if (result == std::future_status::ready) + return true; + { + std::lock_guard lock(*protect_bool); + if (*is_started) { + future_result.wait(); + result = std::future_status::ready; + } else { + *cancel = true; + } + } + return result == std::future_status::ready; }); background_process.set_slicing_completed_event(EVT_SLICING_COMPLETED); background_process.set_finished_event(EVT_PROCESS_COMPLETED); diff --git a/src/slic3r/GUI/Selection.cpp b/src/slic3r/GUI/Selection.cpp index cdd3ebe85..8887e3c31 100644 --- a/src/slic3r/GUI/Selection.cpp +++ b/src/slic3r/GUI/Selection.cpp @@ -447,7 +447,7 @@ void Selection::set_deserialized(EMode mode, const std::vectorset_bounding_boxes_dirty(); } -void Selection::clear() +void Selection::clear(bool is_destroying) { if (!m_valid) return; @@ -466,7 +466,7 @@ void Selection::clear() this->set_bounding_boxes_dirty(); // this happens while the application is closing - if (wxGetApp().obj_manipul() == nullptr) + if (is_destroying || wxGetApp().obj_manipul() == nullptr) return; // resets the cache in the sidebar diff --git a/src/slic3r/GUI/Selection.hpp b/src/slic3r/GUI/Selection.hpp index 4da9e44a3..28c44dc5d 100644 --- a/src/slic3r/GUI/Selection.hpp +++ b/src/slic3r/GUI/Selection.hpp @@ -268,7 +268,7 @@ public: // Update the selection based on the map from old indices to new indices after m_volumes changed. // If the current selection is by instance, this call may select newly added volumes, if they belong to already selected instances. void volumes_changed(const std::vector &map_volume_old_to_new); - void clear(); + void clear(bool is_destroying = false); bool is_empty() const { return m_type == Empty; } bool is_wipe_tower() const { return m_type == WipeTower; }