Merge pull request #1647 from Ultimaker/CURA-9159_alternate_fix_thread_tree_crash

[CURA-9159] Alternate fix crash/deadlocks/threading Tree-support

3 files changed, 16 insertions, 26 deletions

diff --git a/src/TreeModelVolumes.cpp b/src/TreeModelVolumes.cpp
index 1c6889ee8..7479d5a82 100644
--- a/src/TreeModelVolumes.cpp
+++ b/src/TreeModelVolumes.cpp
@@ -104,8 +104,6 @@ const Polygons& TreeModelVolumes::calculateCollision(const RadiusLayerPair& key)
             collision_areas = collision_areas.unionPolygons(collision_model.offset(radius));
         }
     }
-
-    const std::lock_guard<std::mutex> lock(object_mutex_);
     const auto ret = collision_cache_.insert({key, std::move(collision_areas)});
     assert(ret.second);
     return ret.first->second;
@@ -137,8 +135,6 @@ const Polygons& TreeModelVolumes::calculateAvoidance(const RadiusLayerPair& key)
     }
     auto avoidance_areas = getAvoidance(radius, layer_idx - 1).offset(-max_move_).smooth(5);
     avoidance_areas = avoidance_areas.unionPolygons(getCollision(radius, layer_idx));
-
-    const std::lock_guard<std::mutex> lock(object_mutex_);
     const auto ret = avoidance_cache_.insert({key, std::move(avoidance_areas)});
     assert(ret.second);
     return ret.first->second;
@@ -148,9 +144,8 @@ const Polygons& TreeModelVolumes::calculateInternalModel(const RadiusLayerPair&
 {
     const auto& radius = key.first;
     const auto& layer_idx = key.second;
-    const auto& internal_areas = getAvoidance(radius, layer_idx).difference(getCollision(radius, layer_idx));
 
-    const std::lock_guard<std::mutex> lock(object_mutex_);
+    const auto& internal_areas = getAvoidance(radius, layer_idx).difference(getCollision(radius, layer_idx));
     const auto ret = internal_model_cache_.insert({key, internal_areas});
     assert(ret.second);
     return ret.first->second;
diff --git a/src/TreeModelVolumes.h b/src/TreeModelVolumes.h
index 29d23b0d6..74f2b4cc3 100644
--- a/src/TreeModelVolumes.h
+++ b/src/TreeModelVolumes.h
@@ -4,7 +4,6 @@
 #ifndef TREEMODELVOLUMES_H
 #define TREEMODELVOLUMES_H
 
-#include <mutex>
 #include <unordered_map>
 
 #include "settings/EnumSettings.h" //To store whether X/Y or Z distance gets priority.
@@ -21,7 +20,7 @@ class Settings;
 /*!
  * \brief Lazily generates tree guidance volumes.
  *
- * \warning This class blocks on thread access. Use calls to this in threaded blocks sparingly.
+ * \warning This class is not currently thread-safe and should not be accessed in OpenMP blocks
  */
 class TreeModelVolumes
 {
@@ -196,11 +195,6 @@ private:
     mutable std::unordered_map<RadiusLayerPair, Polygons> collision_cache_;
     mutable std::unordered_map<RadiusLayerPair, Polygons> avoidance_cache_;
     mutable std::unordered_map<RadiusLayerPair, Polygons> internal_model_cache_;
-
-    /*!
-     * \brief Used to make the class thread-safe.
-     */
-    mutable std::mutex object_mutex_;
 };
 
 }
diff --git a/src/TreeSupport.cpp b/src/TreeSupport.cpp
index fd6b1f47f..49fac17f9 100644
--- a/src/TreeSupport.cpp
+++ b/src/TreeSupport.cpp
@@ -31,9 +31,10 @@
 namespace cura
 {
 
-TreeSupport::TreeSupport(const SliceDataStorage& storage) :
-    volumes_(storage, Application::getInstance().current_slice->scene.current_mesh_group->settings)
+TreeSupport::TreeSupport(const SliceDataStorage& storage)
 {
+    const Settings& mesh_group_settings = Application::getInstance().current_slice->scene.current_mesh_group->settings;
+    volumes_ = TreeModelVolumes(storage, mesh_group_settings);
 }
 
 void TreeSupport::generateSupportAreas(SliceDataStorage& storage)
@@ -98,8 +99,9 @@ void TreeSupport::drawCircles(SliceDataStorage& storage, const std::vector<std::
     const size_t tip_layers = (branch_radius - minimum_tip_radius) / layer_height; //The number of layers to be shrinking the circle to create a tip. This produces a 45 degree angle.
     const coord_t resolution = mesh_group_settings.get<coord_t>("support_tree_collision_resolution");
 
-    std::atomic<size_t> completed = 0; //To track progress in a multi-threaded environment.
-    std::mutex critical_sections;
+    size_t completed = 0; //To track progress, should be locked when altered.
+    std::mutex critical_section_volumes;
+    std::mutex critical_section_progress;
     cura::parallel_for<size_t>(0, contact_nodes.size(), 1, [&](const size_t layer_nr)
     {
         Polygons support_layer;
@@ -141,8 +143,12 @@ void TreeSupport::drawCircles(SliceDataStorage& storage, const std::vector<std::
         support_layer = support_layer.unionPolygons();
         roof_layer = roof_layer.unionPolygons();
         const size_t z_collision_layer = static_cast<size_t>(std::max(0, static_cast<int>(layer_nr) - static_cast<int>(z_distance_bottom_layers) + 1)); //Layer to test against to create a Z-distance.
-        support_layer = support_layer.difference(volumes_.getCollision(0, z_collision_layer)); //Subtract the model itself (sample 0 is with 0 diameter but proper X/Y offset).
-        roof_layer = roof_layer.difference(volumes_.getCollision(0, z_collision_layer));
+        {
+            const std::lock_guard<std::mutex> lock(critical_section_volumes);
+
+            support_layer = support_layer.difference(volumes_.getCollision(0, z_collision_layer)); //Subtract the model itself (sample 0 is with 0 diameter but proper X/Y offset).
+            roof_layer = roof_layer.difference(volumes_.getCollision(0, z_collision_layer));
+        }
         support_layer = support_layer.difference(roof_layer);
         //We smooth this support as much as possible without altering single circles. So we remove any line less than the side length of those circles.
         const double diameter_angle_scale_factor_this_layer = static_cast<double>(storage.support.supportLayers.size() - layer_nr - tip_layers) * diameter_angle_scale_factor; //Maximum scale factor.
@@ -180,19 +186,14 @@ void TreeSupport::drawCircles(SliceDataStorage& storage, const std::vector<std::
         }
 
         {
-            std::lock_guard<std::mutex> critical_section_support_max_layer_nr(critical_sections);
+            const std::lock_guard<std::mutex> lock(critical_section_progress);
 
             if (!storage.support.supportLayers[layer_nr].support_infill_parts.empty() || !storage.support.supportLayers[layer_nr].support_roof.empty())
             {
                 storage.support.layer_nr_max_filled_layer = std::max(storage.support.layer_nr_max_filled_layer, static_cast<int>(layer_nr));
             }
-        }
-
-        ++completed;
-
-        {
-            std::lock_guard<std::mutex> critical_section_progress(critical_sections);
 
+            ++completed;
             const double progress_contact_nodes = contact_nodes.size() * PROGRESS_WEIGHT_DROPDOWN;
             const double progress_current = completed * PROGRESS_WEIGHT_AREAS;
             const double progress_total = completed * PROGRESS_WEIGHT_AREAS;