BUG: Fix crash in Markups widget when closing scene

When closing the scene, qSlicerMarkupsModuleWidget::updateWidgetFromMRML() would access the subjectHierarchyNode without first checking if it exists, causing a crash.

Fixed by adding a nullptr check.

Also adds additional nullptr checks and warnings to other locations were the subject hierarchy node is accessed without a nullptr check.

See: https://discourse.slicer.org/t/weird-trouble-with-markups/34872/

Modules/Loadable/Markups/qSlicerMarkupsModuleWidget.cxx

@@ -650,11 +650,11 @@ void qSlicerMarkupsModuleWidgetPrivate::setMRMLMarkupsNodeFromSelectionNode()
   // Select current markups node
   vtkMRMLMarkupsNode* currentMarkupsNode = vtkMRMLMarkupsNode::SafeDownCast(this->selectionNodeActivePlaceNode());
 
-  if (!currentMarkupsNode && q->mrmlScene() && this->activeMarkupTreeView->subjectHierarchyNode())
+  vtkMRMLSubjectHierarchyNode* shNode = this->activeMarkupTreeView->subjectHierarchyNode();
+  if (!currentMarkupsNode && q->mrmlScene() && shNode)
   {
     // Active place node is not a markups node then switch to the last markups node.
     vtkCollection* nodes = q->mrmlScene()->GetNodes();
-    vtkMRMLSubjectHierarchyNode* shNode = this->activeMarkupTreeView->subjectHierarchyNode();
     for (int nodeIndex = nodes->GetNumberOfItems() - 1; nodeIndex >= 0; nodeIndex--)
     {
       vtkMRMLMarkupsNode* markupsNode = vtkMRMLMarkupsNode::SafeDownCast(nodes->GetItemAsObject(nodeIndex));
@@ -904,12 +904,13 @@ void qSlicerMarkupsModuleWidget::enter()
   }
 
   // Add event observers to MarkupsNode
-  if (d->MarkupsNode)
+  vtkMRMLSubjectHierarchyNode* shNode = d->activeMarkupTreeView->subjectHierarchyNode();
+  if (d->MarkupsNode && shNode)
   {
     vtkMRMLMarkupsNode* markupsNode = d->MarkupsNode;
     d->MarkupsNode = nullptr; // this will force a reset
     this->setMRMLMarkupsNode(markupsNode);
-    vtkIdType itemID = d->activeMarkupTreeView->subjectHierarchyNode()->GetItemByDataNode(markupsNode);
+    vtkIdType itemID = shNode->GetItemByDataNode(markupsNode);
     QModelIndex itemIndex = d->activeMarkupTreeView->sortFilterProxyModel()->indexFromSubjectHierarchyItem(itemID);
     if(itemIndex.row()>=0)
     {
@@ -967,9 +968,11 @@ void qSlicerMarkupsModuleWidget::updateWidgetFromMRML()
   Q_D(qSlicerMarkupsModuleWidget);
 
   bool wasBlocked = d->activeMarkupTreeView->blockSignals(true);
-  if (d->MarkupsNode)
+
+  vtkMRMLSubjectHierarchyNode* shNode = d->activeMarkupTreeView->subjectHierarchyNode();
+  if (d->MarkupsNode && shNode)
   {
-    vtkIdType itemID = d->activeMarkupTreeView->subjectHierarchyNode()->GetItemByDataNode(d->MarkupsNode);
+    vtkIdType itemID = shNode->GetItemByDataNode(d->MarkupsNode);
     QModelIndex itemIndex = d->activeMarkupTreeView->sortFilterProxyModel()->indexFromSubjectHierarchyItem(itemID);
     if (itemIndex.row() >= 0)
     {

Modules/Loadable/Segmentations/SubjectHierarchyPlugins/qSlicerSubjectHierarchySegmentationsPlugin.cxx

@@ -1075,6 +1075,11 @@ void qSlicerSubjectHierarchySegmentationsPlugin::exportToClosedSurface()
   std::string newFolderName = std::string(segmentationNode->GetName()) + tr("-models")  //: suffix used when exporting segmentation to model
     .toStdString();
   vtkMRMLSubjectHierarchyNode* shNode = qSlicerSubjectHierarchyPluginHandler::instance()->subjectHierarchyNode();
+  if (!shNode)
+  {
+    qCritical() << Q_FUNC_INFO << ": Failed to access subject hierarchy node";
+    return;
+  }
   // Since segmentationNode is not nullptr, we can be sure that shNode is valid.
   vtkIdType segmentationItemID = shNode->GetItemByDataNode(segmentationNode);
   vtkIdType folderItemID = shNode->CreateFolderItem(

Modules/Loadable/Segmentations/SubjectHierarchyPlugins/qSlicerSubjectHierarchySegmentsPlugin.cxx

@@ -729,6 +729,12 @@ void qSlicerSubjectHierarchySegmentsPlugin::showVisibilityContextMenuActionsForI
 
     // Set current segment opacity to the opacity slider
     vtkMRMLSubjectHierarchyNode* shNode = qSlicerSubjectHierarchyPluginHandler::instance()->subjectHierarchyNode();
+   if (!shNode)
+    {
+      qCritical() << Q_FUNC_INFO << ": Failed to access subject hierarchy node";
+      return;
+    }
+
     vtkMRMLScene* scene = qSlicerSubjectHierarchyPluginHandler::instance()->mrmlScene();
     if (!scene)
     {

Modules/Loadable/SubjectHierarchy/Widgets/qMRMLSubjectHierarchyComboBox.cxx

@@ -575,31 +575,38 @@ void qMRMLSubjectHierarchyComboBox::showPopup()
   {
     // If there is no items, find what message to show instead
     vtkMRMLSubjectHierarchyNode* shNode = d->TreeView->subjectHierarchyNode();
-    vtkIdType rootItem = d->TreeView->rootItem();
-    std::vector<vtkIdType> childItemIDs;
-    shNode->GetItemChildren(rootItem, childItemIDs, false);
-    if (childItemIDs.empty())
+    if (shNode)
     {
-      if (rootItem != shNode->GetSceneItemID())
+      vtkIdType rootItem = d->TreeView->rootItem();
+      std::vector<vtkIdType> childItemIDs;
+      shNode->GetItemChildren(rootItem, childItemIDs, false);
+      if (childItemIDs.empty())
       {
-        std::string rootName = shNode->GetItemName(rootItem);
-        QString label = QString("No items in branch: ") + QString::fromStdString(rootName);
-        d->NoItemLabel->setText(label);
+        if (rootItem != shNode->GetSceneItemID())
+        {
+          std::string rootName = shNode->GetItemName(rootItem);
+          QString label = QString("No items in branch: ") + QString::fromStdString(rootName);
+          d->NoItemLabel->setText(label);
+        }
+        else
+        {
+          d->NoItemLabel->setText("No items in scene");
+        }
       }
       else
       {
-        d->NoItemLabel->setText("No items in scene");
+        d->NoItemLabel->setText("No items accepted by current filters");
       }
     }
     else
     {
-      d->NoItemLabel->setText("No items accepted by current filters");
+      d->NoItemLabel->setText("No subject hierarchy");
     }
 
-      // Show no item label instead of tree view
-      d->NoItemLabel->show();
-      d->TreeView->hide();
-      popupHeight = d->NoItemLabel->sizeHint().height();
+    // Show no item label instead of tree view
+    d->NoItemLabel->show();
+    d->TreeView->hide();
+    popupHeight = d->NoItemLabel->sizeHint().height();
   }
   else
   {

Modules/Loadable/SubjectHierarchy/Widgets/qMRMLSubjectHierarchyTreeView.cxx

@@ -523,6 +523,11 @@ vtkMRMLTransformNode* qMRMLSubjectHierarchyTreeViewPrivate::appliedTransformToIt
 //------------------------------------------------------------------------------
 vtkMRMLTransformNode* qMRMLSubjectHierarchyTreeViewPrivate::firstAppliedTransformToSelectedItems()
 {
+  if (!this->SubjectHierarchyNode)
+  {
+    return nullptr;
+  }
+
   QList<vtkIdType> currentItemIDs = this->SelectedItems;
   foreach (vtkIdType itemID, currentItemIDs)
   {
@@ -2261,6 +2266,11 @@ vtkIdType qMRMLSubjectHierarchyTreeView::firstSelectedSubjectHierarchyItemInBran
 {
   Q_D(qMRMLSubjectHierarchyTreeView);
 
+  if (!d->SubjectHierarchyNode)
+  {
+    return vtkMRMLSubjectHierarchyNode::INVALID_ITEM_ID;
+  }
+
   // Check if item itself is selected
   if (d->SelectedItems.contains(itemID))
   {
@@ -2535,6 +2545,12 @@ void qMRMLSubjectHierarchyTreeView::onCustomContextMenu(const QPoint& point)
     return;
   }
 
+  if (!d->SubjectHierarchyNode)
+  {
+    // No subject hierarchy node, ignore the event
+    return;
+  }
+
   if (qSlicerSubjectHierarchyPluginHandler::instance()->LastPluginRegistrationTime > d->LastContextMenuUpdateTime)
   {
     d->setupActions();

Modules/Loadable/SubjectHierarchy/Widgets/qMRMLSubjectHierarchyTreeView.h

@@ -108,6 +108,9 @@ class Q_SLICER_MODULE_SUBJECTHIERARCHY_WIDGETS_EXPORT qMRMLSubjectHierarchyTreeV
 
 public:
   Q_INVOKABLE vtkMRMLScene* mrmlScene()const;
+
+  /// Return the subject hierarchy node found in the current MRML scene.
+  /// While the scene is closing, this method may return a null pointer.
   Q_INVOKABLE vtkMRMLSubjectHierarchyNode* subjectHierarchyNode()const;
 
   /// Get current (=selected) item. If there are multiple items selected, then the first one is returned