Merge pull request #1918 from ghutchis/fix-mo-mesh-races

Fix potential crashes due to race conditions with mesh generation
OpenChemistry · Jan 7, 2025 · f68cc77 · f68cc77
2 parents 87003da + 44227fd
commit f68cc77
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 12 deletions.
diff --git a/avogadro/qtplugins/surfaces/orbitals.cpp b/avogadro/qtplugins/surfaces/orbitals.cpp
@@ -29,7 +29,7 @@ const int smoothingPasses = 1;
 
 Orbitals::Orbitals(QObject* p)
   : ExtensionPlugin(p), m_molecule(nullptr), m_dialog(nullptr),
-    m_action(new QAction(this)), m_runningMutex(new QMutex)
+    m_action(new QAction(this))
 {
   m_action->setEnabled(false);
   m_action->setText(tr("Molecular Orbitals…"));
@@ -45,7 +45,6 @@ Orbitals::~Orbitals()
     if (watcher != nullptr && watcher->isRunning())
       watcher->cancel();
   }
-  delete m_runningMutex;
 
   if (m_dialog)
     m_dialog->deleteLater();
@@ -269,8 +268,9 @@ void Orbitals::addCalculationToQueue(unsigned int orbital, double resolution,
 
 void Orbitals::checkQueue()
 {
-  if (!m_runningMutex->tryLock())
+  if (m_runningCube)
     return;
+  m_runningCube = true;
 
   // Create a hash: keys=priority, values=indices
 
@@ -292,7 +292,7 @@ void Orbitals::checkQueue()
 
   // Do nothing if all calcs are finished.
   if (hash.size() == 0) {
-    m_runningMutex->unlock();
+    m_runningCube = false;
 #ifndef NDEBUG
     qDebug() << "Finished queue.";
 #endif
@@ -392,8 +392,12 @@ void Orbitals::calculateCubeDone()
   watcher->disconnect(this);
 
   if (m_updateMesh) {
-    m_currentMeshCalculation = m_currentRunningCalculation;
-    calculatePosMesh();
+    if (m_currentMeshCalculation == -1) {
+      m_currentMeshCalculation = m_currentRunningCalculation;
+      calculatePosMesh();
+    } else {
+      m_nextMeshCalculation = m_currentRunningCalculation;
+    }
   }
   calculationComplete();
 }
@@ -405,6 +409,7 @@ void Orbitals::calculatePosMesh()
 
   calcInfo* info = &m_queue[m_currentMeshCalculation];
 
+  m_molecule->clearMeshes();
   auto posMesh = m_molecule->addMesh();
   auto cube = info->cube;
 
@@ -458,7 +463,14 @@ void Orbitals::meshComplete()
   if (m_currentMeshCalculation == -1)
     return;
 
-  m_currentMeshCalculation = -1;
+  if (m_nextMeshCalculation != -1) {
+    // start the next mesh calculation
+    m_currentMeshCalculation = m_nextMeshCalculation;
+    m_nextMeshCalculation = -1;
+    calculatePosMesh();
+    return;
+  } else
+    m_currentMeshCalculation = -1;
 }
 
 void Orbitals::calculationComplete()
@@ -472,7 +484,7 @@ void Orbitals::calculationComplete()
 
   info->state = Completed;
   m_currentRunningCalculation = -1;
-  m_runningMutex->unlock();
+  m_runningCube = false;
 
 #ifndef NDEBUG
   qDebug() << info->orbital << " all calculations complete.";
@@ -509,15 +521,19 @@ void Orbitals::renderOrbital(unsigned int row)
   }
 
   // calculate the meshes
-  m_molecule->clearMeshes();
   if (index == -1) {
     // need to calculate the cube first
     calculateOrbitalFromWidget(orbital, OrbitalWidget::OrbitalQualityToDouble(
                                           m_dialog->defaultQuality()));
   } else {
     // just need to update the meshes
-    m_currentMeshCalculation = index;
-    calculatePosMesh(); // will eventually call negMesh too
+    if (m_currentMeshCalculation == -1) {
+      m_currentMeshCalculation = index;
+      calculatePosMesh(); // will eventually call negMesh too
+    } else {
+      // queue it up
+      m_nextMeshCalculation = index;
+    }
   }
 
   // add the orbital to the renderer

diff --git a/avogadro/qtplugins/surfaces/orbitals.h b/avogadro/qtplugins/surfaces/orbitals.h
@@ -148,7 +148,9 @@ private slots:
 
   QList<calcInfo> m_queue;
   int m_currentRunningCalculation = -1;
+  bool m_runningCube = false;
   int m_currentMeshCalculation = -1;
+  int m_nextMeshCalculation = -1;
 
   QtGui::GaussianSetConcurrent* m_gaussianConcurrent = nullptr;
   QtGui::SlaterSetConcurrent* m_slaterConcurrent = nullptr;
@@ -163,7 +165,6 @@ private slots:
 
   OrbitalWidget* m_dialog = nullptr;
   // OrbitalSettingsDialog* m_orbitalSettingsDialog = nullptr;
-  QMutex* m_runningMutex = nullptr;
 };
 
 } // namespace QtPlugins