Fix most memory leaks recently introduced

- Do not overwrite memory allocated in init_mtop() when creating
  molecules in selection tests.
- Add function to properly deallocate t_topology and gmx_mtop_t when
  gmx_mtop_t_to_t_topology(..., false) has been called.
- Free all memory in done_mtop().

These should be enough to (again) make the unit tests free of memory
leaks, except that t_state.enerhist is leaked in a few places.  That
requires more complicated handling.

Change-Id: I84b853b9cb8a11c0458ea4f29316a83a41d7ae22

diff --git a/src/gromacs/fileio/tpxio.cpp b/src/gromacs/fileio/tpxio.cpp
index 4cf763262654896a58f90ad9070a3fad7a0db211..5ba8a74cb93cbfa4bf60c2ddde532d2e444ef66e 100644 (file)
--- a/src/gromacs/fileio/tpxio.cpp
+++ b/src/gromacs/fileio/tpxio.cpp
@@ -3168,7 +3168,7 @@ static int do_tpx(t_fileio *fio, gmx_bool bRead,
         else
         {
             do_mtop(fio, &dum_top, bRead, fileVersion);
-            done_mtop(&dum_top, TRUE);
+            done_mtop(&dum_top);
         }
     }
     do_test(fio, tpx.bX, x);

diff --git a/src/gromacs/gmxpreprocess/grompp.cpp b/src/gromacs/gmxpreprocess/grompp.cpp
index a6ce086301db5ffc109a38cc7a86f0f1bde96d81..e5ab2360cd3dbcd6cf3dda4c4a63e2fd3c8364e3 100644 (file)
--- a/src/gromacs/gmxpreprocess/grompp.cpp
+++ b/src/gromacs/gmxpreprocess/grompp.cpp
@@ -2183,7 +2183,7 @@ int gmx_grompp(int argc, char *argv[])
     write_IMDgroup_to_file(ir->bIMD, ir, &state, sys, NFILE, fnm);
 
     done_atomtype(atype);
-    done_mtop(sys, TRUE);
+    done_mtop(sys);
     done_inputrec_strings();
 
     return 0;

diff --git a/src/gromacs/gmxpreprocess/insert-molecules.cpp b/src/gromacs/gmxpreprocess/insert-molecules.cpp
index 0024aa4fcbba22a5b14c322e271752f50d948a05..936dd144db6889ed0c21aa12f93442ec54363a59 100644 (file)
--- a/src/gromacs/gmxpreprocess/insert-molecules.cpp
+++ b/src/gromacs/gmxpreprocess/insert-molecules.cpp
@@ -339,7 +339,7 @@ class InsertMolecules : public ICommandLineOptionsModule, public ITopologyProvid
         {
             if (top_ != NULL)
             {
-                done_mtop(top_, TRUE);
+                done_mtop(top_);
                 sfree(top_);
             }
         }

diff --git a/src/gromacs/selection/tests/toputils.cpp b/src/gromacs/selection/tests/toputils.cpp
index 224df3a17b2fad52dbc4d30d18bc1c7a506bce10..4de0421cbc02f393b1a0a6c2ac622e1daf9b7e06 100644 (file)
--- a/src/gromacs/selection/tests/toputils.cpp
+++ b/src/gromacs/selection/tests/toputils.cpp
@@ -75,7 +75,7 @@ TopologyManager::~TopologyManager()
 {
     if (mtop_ != nullptr)
     {
-        done_mtop(mtop_, TRUE);
+        done_mtop(mtop_);
         sfree(mtop_);
     }
 
@@ -233,7 +233,7 @@ void TopologyManager::initUniformMolecules(int moleculeSize)
     GMX_RELEASE_ASSERT(mtop_ != NULL, "Topology not initialized");
     int index = 0;
     mtop_->mols.nalloc_index = (mtop_->natoms + moleculeSize - 1) / moleculeSize + 1;
-    snew(mtop_->mols.index, mtop_->mols.nalloc_index);
+    srenew(mtop_->mols.index, mtop_->mols.nalloc_index);
     mtop_->mols.nr = 0;
     while (index < mtop_->natoms)
     {

diff --git a/src/gromacs/topology/mtop_util.cpp b/src/gromacs/topology/mtop_util.cpp
index 76d5fb8d7eff5b7f5d4732634af75d51a5f925c6..9df64226446f4badaefae13c6194e74177c3f06d 100644 (file)
--- a/src/gromacs/topology/mtop_util.cpp
+++ b/src/gromacs/topology/mtop_util.cpp
@@ -968,27 +968,6 @@ gmx_mtop_generate_local_top(const gmx_mtop_t *mtop,
     return top;
 }
 
-static void done_gmx_groups_t(gmx_groups_t *g)
-{
-    int i;
-
-    for (i = 0; (i < egcNR); i++)
-    {
-        if (NULL != g->grps[i].nm_ind)
-        {
-            sfree(g->grps[i].nm_ind);
-            g->grps[i].nm_ind = NULL;
-        }
-        if (NULL != g->grpnr[i])
-        {
-            sfree(g->grpnr[i]);
-            g->grpnr[i] = NULL;
-        }
-    }
-    /* The contents of this array is in symtab, don't free it here */
-    sfree(g->grpname);
-}
-
 t_topology gmx_mtop_t_to_t_topology(gmx_mtop_t *mtop, bool freeMTop)
 {
     int            mt, mb;

diff --git a/src/gromacs/topology/topology.cpp b/src/gromacs/topology/topology.cpp
index f84f1062dd3bc182cf90fd5812bc8f344f9f04d8..27eed7a3c3a2a5d60c8792a495e4015356b5919e 100644 (file)
--- a/src/gromacs/topology/topology.cpp
+++ b/src/gromacs/topology/topology.cpp
@@ -123,12 +123,30 @@ void done_molblock(gmx_molblock_t *molb)
     }
 }
 
-void done_mtop(gmx_mtop_t *mtop, gmx_bool bDoneSymtab)
+void done_gmx_groups_t(gmx_groups_t *g)
 {
-    if (bDoneSymtab)
+    int i;
+
+    for (i = 0; (i < egcNR); i++)
     {
-        done_symtab(&mtop->symtab);
+        if (NULL != g->grps[i].nm_ind)
+        {
+            sfree(g->grps[i].nm_ind);
+            g->grps[i].nm_ind = NULL;
+        }
+        if (NULL != g->grpnr[i])
+        {
+            sfree(g->grpnr[i]);
+            g->grpnr[i] = NULL;
+        }
     }
+    /* The contents of this array is in symtab, don't free it here */
+    sfree(g->grpname);
+}
+
+void done_mtop(gmx_mtop_t *mtop)
+{
+    done_symtab(&mtop->symtab);
 
     sfree(mtop->ffparams.functype);
     sfree(mtop->ffparams.iparams);
@@ -143,6 +161,8 @@ void done_mtop(gmx_mtop_t *mtop, gmx_bool bDoneSymtab)
         done_molblock(&mtop->molblock[i]);
     }
     sfree(mtop->molblock);
+    done_atomtypes(&mtop->atomtypes);
+    done_gmx_groups_t(&mtop->groups);
     done_block(&mtop->mols);
 }
 
@@ -168,6 +188,28 @@ void done_top(t_topology *top)
     done_blocka(&(top->excls));
 }
 
+void done_top_mtop(t_topology *top, gmx_mtop_t *mtop)
+{
+    if (mtop != nullptr)
+    {
+        if (top != nullptr)
+        {
+            for (int f = 0; f < F_NRE; ++f)
+            {
+                sfree(top->idef.il[f].iatoms);
+                top->idef.il[f].iatoms = NULL;
+                top->idef.il[f].nalloc = 0;
+            }
+            done_atom(&top->atoms);
+            done_block(&top->cgs);
+            done_blocka(&top->excls);
+            done_symtab(&top->symtab);
+            open_symtab(&mtop->symtab);
+        }
+        done_mtop(mtop);
+    }
+}
+
 bool gmx_mtop_has_masses(const gmx_mtop_t *mtop)
 {
     if (mtop == nullptr)

diff --git a/src/gromacs/topology/topology.h b/src/gromacs/topology/topology.h
index c9daf7e7f999d36d102b080d746771ca29903e8d..ed2b5ae7de7303bee75e1cbbccfd79585f9186f5 100644 (file)
--- a/src/gromacs/topology/topology.h
+++ b/src/gromacs/topology/topology.h
@@ -147,8 +147,12 @@ void init_mtop(gmx_mtop_t *mtop);
 void init_top(t_topology *top);
 void done_moltype(gmx_moltype_t *molt);
 void done_molblock(gmx_molblock_t *molb);
-void done_mtop(gmx_mtop_t *mtop, gmx_bool bDoneSymtab);
+void done_gmx_groups_t(gmx_groups_t *g);
+void done_mtop(gmx_mtop_t *mtop);
 void done_top(t_topology *top);
+// Frees both t_topology and gmx_mtop_t when the former has been created from
+// the latter.
+void done_top_mtop(t_topology *top, gmx_mtop_t *mtop);
 
 bool gmx_mtop_has_masses(const gmx_mtop_t *mtop);
 bool gmx_mtop_has_charges(const gmx_mtop_t *mtop);

diff --git a/src/gromacs/trajectoryanalysis/analysissettings.cpp b/src/gromacs/trajectoryanalysis/analysissettings.cpp
index 106cc45b8a852595ce6204a1a8d71fc9ec522b12..98759771d41fb60d9ed2de8bfb1c3be285b85287 100644 (file)
--- a/src/gromacs/trajectoryanalysis/analysissettings.cpp
+++ b/src/gromacs/trajectoryanalysis/analysissettings.cpp
@@ -194,12 +194,8 @@ TopologyInformation::TopologyInformation()
 
 TopologyInformation::~TopologyInformation()
 {
-    if (mtop_)
-    {
-        done_mtop(mtop_, TRUE);
-        sfree(mtop_);
-    }
-    // TODO: Free that part of the memory that is not shared with mtop_.
+    done_top_mtop(top_, mtop_);
+    sfree(mtop_);
     sfree(top_);
     sfree(xtop_);
 }