Merge pull request #155 from dash-project/bug-140-dart-memleaks

Fixes #140

dart-impl/base/include/dash/dart/base/internal/host_topology.h

@@ -31,6 +31,7 @@ typedef struct
   int                       num_nodes;
   int                       num_hosts;
   int                       num_host_levels;
+  size_t                    num_units;
   char **                   host_names;
   dart_host_units_t       * host_units;
   dart_host_domain_t      * host_domains;

dart-impl/base/src/internal/host_topology.c

@@ -160,8 +160,8 @@ dart_ret_t dart__base__host_topology__update_module_locations(
   dart_unit_locality_t * my_uloc;
   dart_unit_t            local_leader_unit_id = DART_UNDEFINED_UNIT_ID;
   dart_unit_t            my_id                = DART_UNDEFINED_UNIT_ID;
-  dart_group_t         * leader_group         = malloc(sizeof(group_t_size));
-  dart_group_t         * local_group          = malloc(sizeof(group_t_size));
+  dart_group_t         * leader_group         = malloc(group_t_size);
+  dart_group_t         * local_group          = malloc(group_t_size);
   dart_team_t            leader_team; /* team of all node leaders   */
 
   DART_ASSERT_RETURNS(
@@ -673,6 +673,7 @@ dart_ret_t dart__base__host_topology__create(
   topo->num_host_levels = 0;
   topo->num_nodes       = num_hosts;
   topo->num_hosts       = num_hosts;
+  topo->num_units       = num_units;
   topo->host_names      = hostnames;
 
   DART_ASSERT_RETURNS(
@@ -702,7 +703,7 @@ dart_ret_t dart__base__host_topology__destruct(
     topo->host_domains = NULL;
   }
   if (NULL != topo->host_names) {
-    for (int h = 0; h < topo->num_hosts; ++h) {
+    for (size_t h = 0; h < topo->num_units; ++h) {
       if (NULL != topo->host_names[h]) {
         DART_LOG_DEBUG("dart__base__host_topology__init: "
                        "free(topo->host_names[%d])", h);

dart-impl/base/src/locality.c

@@ -217,12 +217,6 @@ dart_ret_t dart__base__locality__delete(
 {
   dart_ret_t ret = DART_OK;
 
-  if (NULL == dart__base__locality__global_domain_[team] &&
-      NULL == dart__base__locality__host_topology_[team] &&
-      NULL == dart__base__locality__unit_mapping_[team]) {
-    return ret;
-  }
-
   DART_LOG_DEBUG("dart__base__locality__delete() team(%d)", team);
 
   if (NULL != dart__base__locality__global_domain_[team]) {

dart-impl/mpi/src/dart_initialization.c

@@ -305,6 +305,7 @@ dart_ret_t dart_exit()
 	MPI_Win_free(&dart_sharedmem_win_local_alloc);
 #endif
   MPI_Win_free(&team_data->window);
+  MPI_Comm_free(&(team_data->sharedmem_comm));
 
   /* <fuchsto>: Why calling dart_segment_clear twice? */
 /*
@@ -315,6 +316,7 @@ dart_ret_t dart_exit()
   free(team_data->sharedmem_tab);
   free(dart_sharedmem_local_baseptr_set);
 #endif
+
 	dart_adapt_teamlist_destroy();
 
   /* <fuchsto>: deactivated, currently segfaults when running

dart-impl/mpi/src/dart_team_group.c

@@ -28,22 +28,28 @@
 dart_ret_t dart_group_init(
   dart_group_t *group)
 {
-  group -> mpi_group = MPI_GROUP_EMPTY;
+  // Initialize the group as empty but not directly assign MPI_GROUP_EMPTY as it might lead to invalid free later
+  MPI_Group g;
+  MPI_Comm_group(MPI_COMM_WORLD, &g);
+  MPI_Group_incl(g, 0, NULL, &group->mpi_group);
   return DART_OK;
 }
 
 dart_ret_t dart_group_fini(
   dart_group_t *group)
 {
-  group -> mpi_group = MPI_GROUP_NULL;
+  if (group->mpi_group != MPI_GROUP_NULL) {
+    MPI_Group_free(&group->mpi_group);
+    group->mpi_group = MPI_GROUP_NULL;
+  }
   return DART_OK;
 }
 
 dart_ret_t dart_group_copy(
   const dart_group_t *gin,
   dart_group_t *gout)
 {
-  gout -> mpi_group = gin -> mpi_group;
+  gout->mpi_group = gin -> mpi_group;
   return DART_OK;
 }
 
@@ -101,7 +107,7 @@ dart_ret_t dart_group_union(
       while (j <= size_out -1) {
         post_unitidsout[k++] = pre_unitidsout[j++];
       }
-      gout -> mpi_group = MPI_GROUP_EMPTY;
+      MPI_Group_free(&gout->mpi_group);
       MPI_Group_incl(
         group_all,
         size_out,
@@ -143,17 +149,18 @@ dart_ret_t dart_group_addmember(
 {
   int array[1];
   dart_group_t group_copy, group;
-  MPI_Group     newgroup, group_all;
+  MPI_Group     group_all;
   /* Group_all comprises all the running units. */
   MPI_Comm_group(MPI_COMM_WORLD, &group_all);
 //  group_copy = (dart_group_t *)malloc(sizeof(dart_group_t));
 //  group      = (dart_group_t *)malloc(sizeof(dart_group_t));
   dart_group_copy(g, &group_copy);
   array[0]   = unitid;
-  MPI_Group_incl(group_all, 1, array, &newgroup);
-  group.mpi_group = newgroup;
+  MPI_Group_incl(group_all, 1, array, &group.mpi_group);
   /* Make the new group being an ordered group. */
   dart_group_union(&group_copy, &group, g);
+  dart_group_fini(&group);
+  dart_group_fini(&group_copy);
   return DART_OK;
 }
 
@@ -174,6 +181,7 @@ dart_ret_t dart_group_delmember(
     g -> mpi_group,
     newgroup,
     &(g -> mpi_group));
+  MPI_Group_free(&newgroup);
   return DART_OK;
 }
 
@@ -248,7 +256,7 @@ dart_ret_t dart_group_split(
         &grouptem);
       (*(gout + i))->mpi_group = grouptem;
     } else {
-      (*(gout + i))->mpi_group = MPI_GROUP_EMPTY;
+      (*(gout + i))->mpi_group = MPI_GROUP_NULL;
     }
   }
   return DART_OK;

dash/test/DARTLocalityTest.cc

@@ -123,8 +123,8 @@ TEST_F(DARTLocalityTest, ExcludeLocalityDomain)
 
   // Remove the active unit's domain:
   const char * excluded_domain = ul->domain_tag;
-  dart_domain_exclude(
-    loc_team_all_copy, 1, &excluded_domain);
+  EXPECT_EQ_U(DART_OK, dart_domain_exclude(
+    loc_team_all_copy, 1, &excluded_domain));
 
   // Lookup of excluded domain should fail and return null pointer:
   dart_domain_locality_t * no_domain;