fix an occasional SLiMgui crash involving removed subpops

Author: bhaller

QtSLiM/QtSLiMPopulationTable.cpp

@@ -68,13 +68,24 @@ QVariant QtSLiMPopulationTableModel::data(const QModelIndex &p_index, int role)
 
             std::advance(popIter, p_index.row());
             Subpopulation *subpop = *popIter;
+            Species *species = &(subpop->species_);
+            
+            // BCH 3/21/2024: This check is a debugging leftover that should stay permanently.  The display list
+            // for the population table was out of date and contained subpopulations that had been deallocated,
+            // leading to a crash.  This was a very hard bug to find, so it's worth keeping this code here.  The
+            // bug was fixed with needsUpdateForDisplaySubpops() in QtSLiMPopulationTableModel.  
+            if (species == nullptr)
+            {
+                qDebug() << "INVALID SUBPOPULATION in QtSLiMPopulationTableModel::data()!";
+                qApp->beep();
+            }
 
             if (p_index.column() == 0)
             {
                 QString idString = QString("p%1").arg(subpop->subpopulation_id_);
 
                 if (community->all_species_.size() > 1)
-                    idString.append(" ").append(QString::fromStdString(subpop->species_.avatar_));
+                    idString.append(" ").append(QString::fromStdString(species->avatar_));
 
                 return QVariant(idString);
             }
@@ -210,17 +221,19 @@ QVariant QtSLiMPopulationTableModel::headerData(int section,
     return QVariant();
 }
 
-void QtSLiMPopulationTableModel::reloadTable(void)
+bool QtSLiMPopulationTableModel::needsUpdateForDisplaySubpops(std::vector<Subpopulation *> &newDisplayList)
+{
+    // Checks whether our cached display list is out of date; if it is, a reload needs to be forced.
+    return (displaySubpops != newDisplayList);
+}
+
+void QtSLiMPopulationTableModel::reloadTable(std::vector<Subpopulation *> &newDisplayList)
 {
     beginResetModel();
 
     // recache the list of subpopulations we display
-    displaySubpops.clear();
-    
-    QtSLiMWindow *controller = static_cast<QtSLiMWindow *>(parent());
-    
-    if (controller)
-        displaySubpops = controller->listedSubpopulations();
+    std::swap(displaySubpops, newDisplayList);
+    newDisplayList.clear();
 
     endResetModel();
 }

QtSLiM/QtSLiMPopulationTable.h

@@ -42,7 +42,8 @@ class QtSLiMPopulationTableModel : public QAbstractTableModel
     virtual QVariant data(const QModelIndex &index, int role = Qt::DisplayRole) const override;
     virtual QVariant headerData(int section, Qt::Orientation p_orientation, int role = Qt::DisplayRole) const override;
 
-    void reloadTable(void);
+    bool needsUpdateForDisplaySubpops(std::vector<Subpopulation *> &newDisplayList);
+    void reloadTable(std::vector<Subpopulation *> &newDisplayList);
 
     Subpopulation *subpopAtIndex(int i) const { return displaySubpops[i]; }
 

QtSLiM/QtSLiMWindow.cpp

@@ -2612,17 +2612,25 @@ void QtSLiMWindow::updateAfterTickFull(bool fullUpdate)
 	// The rest of the code here needs to be careful about the invalid state; we do want to update our controls when invalid, but sim is nil.
     bool inInvalidState = (!community || !community->simulation_valid_ || invalidSimulation());
 
-	if (fullUpdate)
+    if (fullUpdate)
+        updateOutputViews();
+    
+    // Minimal population table updating.  When the list of subpops changes, we always need to redisplay, otherwise the display
+    // list is outdated and might contain deallocated Subpopulations.  Other than that, though, we only want to redisplay
+    // on full updates, because reloading and redisplaying the tableview can be quite expensive.  The QtSLiMPopulationTableModel
+    // keeps a cache of its display list, and we can use that to see whether a redisplay is needed or not.
+    std::vector<Subpopulation *> newDisplaySubpops = listedSubpopulations();
+    
+    if (fullUpdate || populationTableModel_->needsUpdateForDisplaySubpops(newDisplaySubpops))
 	{
-		// FIXME it would be good for this updating to be minimal; reloading the tableview every time, etc., is quite wasteful...
-		updateOutputViews();
-		
+        //qDebug() << "UPDATING TABLE";
+        
 		// Reloading the subpop tableview is tricky, because we need to preserve the selection across the reload, while also noting that the selection is forced
 		// to change when a subpop goes extinct.  The current selection is noted in the gui_selected_ ivar of each subpop.  So what we do here is reload the tableview
 		// while suppressing our usual update of our selection state, and then we try to re-impose our selection state on the new tableview content.  If a subpop
 		// went extinct, we will fail to notice the selection change; but that is OK, since we force an update of populationView and chromosomeZoomed below anyway.
-		reloadingSubpopTableview = true;
-        populationTableModel_->reloadTable();
+		reloadingSubpopTableview = true;                        // suppresses QtSLiMWindow::subpopSelectionDidChange()
+        populationTableModel_->reloadTable(newDisplaySubpops);  // invalidates newDisplaySubpops with std::swap()
 
         int subpopCount = populationTableModel_->rowCount();
 
@@ -2654,6 +2662,10 @@ void QtSLiMWindow::updateAfterTickFull(bool fullUpdate)
         if ((ui->subpopTableView->selectionModel()->selectedRows().size() == 0) && subpopCount)
             ui->subpopTableView->selectAll();
 	}
+    else
+    {
+        //qDebug() << "skipping unnecessary table update";
+    }
 
 	// Now update our other UI, some of which depends upon the state of subpopTableView
     ui->individualsWidget->update();