FIX: Fix Linux segfault on clear scene

- Fix segfault when clearing scene due to std::map erase in for range loop

Author: Thibault-Pelletier

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineManager.cxx

@@ -282,6 +282,21 @@ vtkSmartPointer<vtkMRMLLayerDMPipelineI> vtkMRMLLayerDMPipelineManager::GetNodeP
   return found->second;
 }
 
+int vtkMRMLLayerDMPipelineManager::GetNumberOfPipelines() const
+{
+  return this->m_pipelineMap.size();
+}
+
+vtkMRMLLayerDMPipelineI* vtkMRMLLayerDMPipelineManager::GetNthPipeline(int iPipeline) const
+{
+  if (iPipeline < 0 || iPipeline >= this->m_pipelineMap.size())
+  {
+    return nullptr;
+  }
+
+  return std::next(this->m_pipelineMap.begin(), iPipeline)->second;
+}
+
 void vtkMRMLLayerDMPipelineManager::SetRenderer(vtkRenderer* renderer) const
 {
   // Pass the renderer to the camera sync
@@ -306,13 +321,19 @@ void vtkMRMLLayerDMPipelineManager::RemoveOutdatedPipelines()
     return;
   }
 
+  std::vector<vtkWeakPointer<vtkMRMLNode>> outdatedPipelines;
   for (const auto& pipe : m_pipelineMap)
   {
     if (!pipe.first || !this->m_scene->GetNodeByID(pipe.first->GetID()))
     {
-      this->RemovePipeline(pipe.first);
+      outdatedPipelines.emplace_back(pipe.first);
     }
   }
+
+  for (const auto& pipe : outdatedPipelines)
+  {
+    this->RemovePipeline(pipe);
+  }
 }
 
 void vtkMRMLLayerDMPipelineManager::AddMissingPipelines()

LayerDM/MRMLDM/vtkMRMLLayerDMPipelineManager.h

@@ -63,6 +63,14 @@ class VTK_SLICER_LAYERDM_MODULE_MRMLDISPLAYABLEMANAGER_EXPORT vtkMRMLLayerDMPipe
   /// Returns the pipeline associated with the input display node if any.
   vtkSmartPointer<vtkMRMLLayerDMPipelineI> GetNodePipeline(vtkMRMLNode* node) const;
 
+  /// Returns the number of pipelines currently managed by the pipeline manager
+  int GetNumberOfPipelines() const;
+
+  /// Returns the list of currently managed display nodes of the pipeline manager.
+  ///
+  /// \sa GetNodePipeline
+  vtkMRMLLayerDMPipelineI* GetNthPipeline(int iPipeline) const;
+
   /// @{
   /// Makes the latest pipeline lose focus
   void LoseFocus() const;

LayerDM/Testing/Python/PipelineManagerTest.py

@@ -263,3 +263,36 @@ def test_on_pipeline_removed_triggers_renderer_removed(self):
 
         self.pipelineManager.RemoveNode(m1.GetDisplayNode())
         m1.mockOnRendererRemoved.assert_called_once_with(self.defaultRenderer)
+
+    def test_correctly_cleans_up_outdated_pipelines(self):
+        modelNodes = [slicer.mrmlScene.AddNewNodeByClass("vtkMRMLModelNode") for _ in range(5)]
+        for modelNode in modelNodes:
+            self.pipelineManager.AddNode(modelNode)
+        pipelines = [self.pipelineManager.GetNthPipeline(i_pipe) for i_pipe in range(self.pipelineManager.GetNumberOfPipelines())]
+        modelPipelines = [p for p in pipelines if p.GetDisplayNode() in modelNodes]
+        assert len(modelPipelines) == 5
+        prevNumber = self.pipelineManager.GetNumberOfPipelines()
+        slicer.mrmlScene.RemoveNode(modelNodes[0])
+        slicer.mrmlScene.RemoveNode(modelNodes[1])
+        slicer.mrmlScene.RemoveNode(modelNodes[2])
+        assert self.pipelineManager.GetNumberOfPipelines() == prevNumber
+        self.pipelineManager.UpdateFromScene()
+        assert self.pipelineManager.GetNumberOfPipelines() == prevNumber - 3
+
+    def test_notifies_pipelines_when_references_are_added_or_removed(self):
+        # Create a display markups node
+        m1 = MockPipeline()
+        self.nextMock = m1
+        dNode = slicer.mrmlScene.AddNewNodeByClass("vtkMRMLMarkupsFiducialDisplayNode")
+        assert self.pipelineManager.AddNode(dNode)
+
+        # Create a markups node
+        markups = slicer.mrmlScene.AddNewNodeByClass("vtkMRMLMarkupsFiducialNode")
+
+        # Set the markups node display to the previous display node and expect the pipeline to have been notified
+        markups.SetAndObserveDisplayNodeID(dNode.GetID())
+        m1.mockOnReferenceToDisplayNodeAdded.assert_called_once_with(markups, "display")
+
+        # Unset the display node and expect the pipeline to have been notified
+        markups.SetAndObserveDisplayNodeID("")
+        m1.mockOnReferenceToDisplayNodeRemoved.assert_called_once_with(markups, "display")