Fixed segfault when deleting the Context and Integrator in the wrong order

openmmapi/include/openmm/BrownianIntegrator.h

@@ -132,8 +132,6 @@ protected:
 private:
     double temperature, friction;
     int randomNumberSeed;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/CustomIntegrator.h

@@ -473,8 +473,6 @@ private:
     mutable bool globalsAreCurrent;
     int randomNumberSeed;
     bool forcesAreValid;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/Integrator.h

@@ -37,6 +37,7 @@
 #include <map>
 #include <vector>
 #include "internal/windowsExport.h"
+#include "internal/ContextImpl.h"
 
 namespace OpenMM {
 
@@ -53,8 +54,17 @@ class ContextImpl;
 
 class OPENMM_EXPORT Integrator {
 public:
+    Integrator() : owner(NULL), context(NULL) {
+    }
     virtual ~Integrator() {
-        cleanup();
+        if (context != NULL) {
+            // The Integrator is being deleted before the Context, so do cleanup now,
+            // then notify the ContextImpl so its own destructor won't try to clean up
+            // the (no longer existing) Integrator.
+            
+            cleanup();
+            context->integratorDeleted();
+        }
     }
     /**
      * Get the size of each time step, in picoseconds.  If this integrator uses variable time steps,
@@ -95,6 +105,8 @@ public:
 protected:
     friend class Context;
     friend class ContextImpl;
+    ContextImpl* context;
+    Context* owner;
     /**
      * This will be called by the Context when it is created.  It informs the Integrator
      * of what context it will be integrating, and gives it a chance to do any necessary initialization.

openmmapi/include/openmm/LangevinIntegrator.h

@@ -132,8 +132,6 @@ protected:
 private:
     double temperature, friction;
     int randomNumberSeed;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/VariableLangevinIntegrator.h

@@ -163,8 +163,6 @@ protected:
 private:
     double temperature, friction, errorTol;
     int randomNumberSeed;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/VariableVerletIntegrator.h

@@ -115,8 +115,6 @@ protected:
     double computeKineticEnergy();
 private:
     double errorTol;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/VerletIntegrator.h

@@ -77,8 +77,6 @@ protected:
      */
     double computeKineticEnergy();
 private:
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

openmmapi/include/openmm/internal/ContextImpl.h

@@ -243,6 +243,13 @@ public:
      * @param stream    an input stream the checkpoint data should be read from
      */
     void loadCheckpoint(std::istream& stream);
+    /**
+     * This is invoked by the Integrator when it is deleted.  This is needed to ensure the cleanup process
+     * is done correctly, since we don't know whether the Integrator or Context will be deleted first.
+     */
+    void integratorDeleted() {
+        integratorIsDeleted = true;
+    }
 private:
     friend class Context;
     static void tagParticlesInMolecule(int particle, int molecule, std::vector<int>& particleMolecule, std::vector<std::vector<int> >& particleBonds);
@@ -252,7 +259,7 @@ private:
     std::vector<ForceImpl*> forceImpls;
     std::map<std::string, double> parameters;
     mutable std::vector<std::vector<int> > molecules;
-    bool hasInitializedForces, hasSetPositions;
+    bool hasInitializedForces, hasSetPositions, integratorIsDeleted;
     int lastForceGroups;
     Platform* platform;
     Kernel initializeForcesKernel, updateStateDataKernel, applyConstraintsKernel, virtualSitesKernel;

openmmapi/src/BrownianIntegrator.cpp

@@ -41,7 +41,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-BrownianIntegrator::BrownianIntegrator(double temperature, double frictionCoeff, double stepSize) : owner(NULL) {
+BrownianIntegrator::BrownianIntegrator(double temperature, double frictionCoeff, double stepSize) {
     setTemperature(temperature);
     setFriction(frictionCoeff);
     setStepSize(stepSize);

openmmapi/src/ContextImpl.cpp

@@ -48,7 +48,8 @@ using namespace OpenMM;
 using namespace std;
 
 ContextImpl::ContextImpl(Context& owner, System& system, Integrator& integrator, Platform* platform, const map<string, string>& properties) :
-         owner(owner), system(system), integrator(integrator), hasInitializedForces(false), hasSetPositions(false), lastForceGroups(-1), platform(platform), platformData(NULL) {
+        owner(owner), system(system), integrator(integrator), hasInitializedForces(false), hasSetPositions(false), integratorIsDeleted(false),
+        lastForceGroups(-1), platform(platform), platformData(NULL) {
     if (system.getNumParticles() == 0)
         throw OpenMMException("Cannot create a Context for a System with no particles");
     
@@ -122,6 +123,12 @@ ContextImpl::~ContextImpl() {
     updateStateDataKernel = Kernel();
     applyConstraintsKernel = Kernel();
     virtualSitesKernel = Kernel();
+    if (!integratorIsDeleted) {
+        // The Context is being deleted before the Integrator, so call cleanup() on it now.
+        
+        integrator.cleanup();
+        integrator.context = NULL;
+    }
     platform->contextDestroyed(*this);
 }
 

openmmapi/src/CustomIntegrator.cpp

@@ -42,7 +42,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-CustomIntegrator::CustomIntegrator(double stepSize) : owner(NULL), globalsAreCurrent(true), forcesAreValid(false) {
+CustomIntegrator::CustomIntegrator(double stepSize) : globalsAreCurrent(true), forcesAreValid(false) {
     setStepSize(stepSize);
     setConstraintTolerance(1e-4);
     setRandomNumberSeed((int) time(NULL));

openmmapi/src/LangevinIntegrator.cpp

@@ -41,7 +41,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-LangevinIntegrator::LangevinIntegrator(double temperature, double frictionCoeff, double stepSize) : owner(NULL) {
+LangevinIntegrator::LangevinIntegrator(double temperature, double frictionCoeff, double stepSize) {
     setTemperature(temperature);
     setFriction(frictionCoeff);
     setStepSize(stepSize);

openmmapi/src/VariableLangevinIntegrator.cpp

@@ -42,7 +42,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-VariableLangevinIntegrator::VariableLangevinIntegrator(double temperature, double frictionCoeff, double errorTol) : owner(NULL) {
+VariableLangevinIntegrator::VariableLangevinIntegrator(double temperature, double frictionCoeff, double errorTol) {
     setTemperature(temperature);
     setFriction(frictionCoeff);
     setErrorTolerance(errorTol);

openmmapi/src/VariableVerletIntegrator.cpp

@@ -41,7 +41,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-VariableVerletIntegrator::VariableVerletIntegrator(double errorTol) : errorTol(errorTol), owner(NULL) {
+VariableVerletIntegrator::VariableVerletIntegrator(double errorTol) : errorTol(errorTol) {
     setConstraintTolerance(1e-4);
 }
 

openmmapi/src/VerletIntegrator.cpp

@@ -40,7 +40,7 @@ using namespace OpenMM;
 using std::string;
 using std::vector;
 
-VerletIntegrator::VerletIntegrator(double stepSize) : owner(NULL) {
+VerletIntegrator::VerletIntegrator(double stepSize) {
     setStepSize(stepSize);
     setConstraintTolerance(1e-4);
 }

plugins/rpmd/openmmapi/include/openmm/RPMDIntegrator.h

@@ -183,8 +183,6 @@ private:
     double temperature, friction;
     int numCopies, randomNumberSeed;
     bool forcesAreValid, hasSetPosition, hasSetVelocity, isFirstStep;
-    ContextImpl* context;
-    Context* owner;
     Kernel kernel;
 };
 

plugins/rpmd/openmmapi/src/RPMDIntegrator.cpp

@@ -42,7 +42,7 @@ using std::string;
 using std::vector;
 
 RPMDIntegrator::RPMDIntegrator(int numCopies, double temperature, double frictionCoeff, double stepSize) :
-        owner(NULL), numCopies(numCopies), forcesAreValid(false), hasSetPosition(false), hasSetVelocity(false), isFirstStep(true) {
+        numCopies(numCopies), forcesAreValid(false), hasSetPosition(false), hasSetVelocity(false), isFirstStep(true) {
     setTemperature(temperature);
     setFriction(frictionCoeff);
     setStepSize(stepSize);