Merge pull request #457 from hjelmn/mpit_fixes

mca/base: fix bugs in framework deregistration/re-registration

ompi/mpi/tool/finalize.c

@@ -1,6 +1,6 @@
 /* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
 /*
- * Copyright (c) 2012-2013 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * Copyright (c) 2014 Cisco Systems, Inc.  All rights reserved.
  * $COPYRIGHT$
@@ -13,6 +13,7 @@
 
 #include "ompi/mpi/tool/mpit-internal.h"
 
+#include "ompi/runtime/ompi_info_support.h"
 #include "opal/include/opal/sys/atomic.h"
 #include "opal/runtime/opal.h"
 
@@ -35,6 +36,7 @@ int MPI_T_finalize (void)
     }
 
     if (0 == --mpit_init_count) {
+        (void) ompi_info_close_components ();
         (void) opal_finalize_util ();
     }
 

opal/mca/base/mca_base_framework.c

@@ -24,6 +24,11 @@ static bool framework_is_registered (struct mca_base_framework_t *framework)
     return !!(framework->framework_flags & MCA_BASE_FRAMEWORK_FLAG_REGISTERED);
 }
 
+static bool framework_is_open (struct mca_base_framework_t *framework)
+{
+    return !!(framework->framework_flags & MCA_BASE_FRAMEWORK_FLAG_OPEN);
+}
+
 static void framework_open_output (struct mca_base_framework_t *framework)
 {
     if (0 < framework->framework_verbose) {
@@ -54,6 +59,8 @@ int mca_base_framework_register (struct mca_base_framework_t *framework,
 
     assert (NULL != framework);
 
+    framework->framework_refcnt++;
+
     if (framework_is_registered (framework)) {
         return OPAL_SUCCESS;
     }
@@ -127,17 +134,17 @@ int mca_base_framework_open (struct mca_base_framework_t *framework,
 
     assert (NULL != framework);
 
-    /* check if this framework is already open */
-    if (framework->framework_refcnt++) {
-        return OPAL_SUCCESS;
-    }
-
     /* register this framework before opening it */
     ret = mca_base_framework_register (framework, MCA_BASE_REGISTER_DEFAULT);
     if (OPAL_SUCCESS != ret) {
         return ret;
     }
 
+    /* check if this framework is already open */
+    if (framework_is_open (framework)) {
+        return OPAL_SUCCESS;
+    }
+
     if (MCA_BASE_FRAMEWORK_FLAG_NOREGISTER & framework->framework_flags) {
         flags |= MCA_BASE_OPEN_FIND_COMPONENTS;
     }
@@ -158,23 +165,27 @@ int mca_base_framework_open (struct mca_base_framework_t *framework,
     }
 
     if (OPAL_SUCCESS != ret) {
-        framework->framework_refcnt = 0;
+        framework->framework_refcnt--;
+    } else {
+        framework->framework_flags |= MCA_BASE_FRAMEWORK_FLAG_OPEN;
     }
 
     return ret;
 }
 
 int mca_base_framework_close (struct mca_base_framework_t *framework) {
-    bool is_open = !!framework->framework_refcnt;
+    bool is_open = framework_is_open (framework);
+    bool is_registered = framework_is_registered (framework);
     int ret, group_id;
 
     assert (NULL != framework);
 
-    if (!framework_is_registered (framework) && 0 == framework->framework_refcnt) {
+    if (!(is_open || is_registered)) {
         return OPAL_SUCCESS;
     }
 
-    if (framework->framework_refcnt && --framework->framework_refcnt) {
+    assert (framework->framework_refcnt);
+    if (--framework->framework_refcnt) {
         return OPAL_SUCCESS;
     }
 
@@ -183,7 +194,6 @@ int mca_base_framework_close (struct mca_base_framework_t *framework) {
                                         framework->framework_name, NULL);
     if (0 <= group_id) {
         (void) mca_base_var_group_deregister (group_id);
-        framework->framework_flags &= ~MCA_BASE_FRAMEWORK_FLAG_REGISTERED;
     }
 
     /* close the framework and all of its components */
@@ -209,7 +219,7 @@ int mca_base_framework_close (struct mca_base_framework_t *framework) {
         ret = OPAL_SUCCESS;
     }
 
-    framework->framework_flags &= ~MCA_BASE_FRAMEWORK_FLAG_REGISTERED;
+    framework->framework_flags &= ~(MCA_BASE_FRAMEWORK_FLAG_REGISTERED | MCA_BASE_FRAMEWORK_FLAG_OPEN);
 
     framework_close_output (framework);
 

opal/mca/base/mca_base_framework.h

@@ -114,7 +114,8 @@ typedef enum {
     MCA_BASE_FRAMEWORK_FLAG_REGISTERED = 2,
     /** Framework does not have any DSO components */
     MCA_BASE_FRAMEWORK_FLAG_NO_DSO     = 4,
-
+    /** Internal. Don't set outside mca_base_framework.h */
+    MCA_BASE_FRAMEWORK_FLAG_OPEN       = 8,
     /**
      * The upper 16 bits are reserved for project specific flags.
      */

opal/mca/base/mca_base_var.c

@@ -11,7 +11,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2008-2015 Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2012-2014 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * Copyright (c) 2014      Intel, Inc. All rights reserved.
  * Copyright (c) 2015      Research Organization for Information Science
@@ -1369,6 +1369,10 @@ static int register_variable (const char *project_name, const char *framework_na
             return OPAL_ERROR;
         }
 
+        if (!group->group_isvalid) {
+            group->group_isvalid = true;
+        }
+
         /* Verify the name components match */
         if (0 != compare_strings(framework_name, group->group_framework) ||
             0 != compare_strings(component_name, group->group_component) ||

opal/mca/base/mca_base_var_group.c

@@ -11,7 +11,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2008-2013 Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2012-2013 Los Alamos National Security, LLC. All rights
+ * Copyright (c) 2012-2015 Los Alamos National Security, LLC. All rights
  *                         reserved.
  * $COPYRIGHT$
  * 
@@ -131,7 +131,7 @@ static int group_find_by_name (const char *full_name, int *index, bool invalidok
         return rc;
     }
 
-    rc = mca_base_var_group_get_internal ((int)(uintptr_t) tmp, &group, false);
+    rc = mca_base_var_group_get_internal ((int)(uintptr_t) tmp, &group, invalidok);
     if (OPAL_SUCCESS != rc) {
         return rc;
     }
@@ -291,7 +291,7 @@ int mca_base_var_group_component_register (const mca_base_component_t *component
 int mca_base_var_group_deregister (int group_index)
 {
     mca_base_var_group_t *group;
-    int size, i, ret;
+    int size, ret;
     int *params, *subgroups;
 
     ret = mca_base_var_group_get_internal (group_index, &group, false);
@@ -305,7 +305,7 @@ int mca_base_var_group_deregister (int group_index)
     size = opal_value_array_get_size(&group->group_vars);
     params = OPAL_VALUE_ARRAY_GET_BASE(&group->group_vars, int);
 
-    for (i = 0 ; i < size ; ++i) {
+    for (int i = 0 ; i < size ; ++i) {
         const mca_base_var_t *var;
 
         ret = mca_base_var_get (params[i], &var);
@@ -315,14 +315,12 @@ int mca_base_var_group_deregister (int group_index)
 
         (void) mca_base_var_deregister (params[i]);
     }
-    OBJ_DESTRUCT(&group->group_vars);
-    OBJ_CONSTRUCT(&group->group_vars, opal_value_array_t);
 
     /* invalidate all associated mca performance variables */
     size = opal_value_array_get_size(&group->group_pvars);
     params = OPAL_VALUE_ARRAY_GET_BASE(&group->group_pvars, int);
 
-    for (i = 0 ; i < size ; ++i) {
+    for (int i = 0 ; i < size ; ++i) {
         const mca_base_pvar_t *var;
 
         ret = mca_base_pvar_get (params[i], &var);
@@ -332,16 +330,14 @@ int mca_base_var_group_deregister (int group_index)
 
         (void) mca_base_pvar_mark_invalid (params[i]);
     }
-    OBJ_DESTRUCT(&group->group_pvars);
-    OBJ_CONSTRUCT(&group->group_pvars, opal_value_array_t);
 
     size = opal_value_array_get_size(&group->group_subgroups);
     subgroups = OPAL_VALUE_ARRAY_GET_BASE(&group->group_subgroups, int);
-    for (i = 0 ; i < size ; ++i) {
+    for (int i = 0 ; i < size ; ++i) {
         (void) mca_base_var_group_deregister (subgroups[i]);
     }
-    OBJ_DESTRUCT(&group->group_subgroups);
+    /* ordering of variables and subgroups must be the same if the
-    OBJ_CONSTRUCT(&group->group_subgroups, opal_value_array_t);
+     * group is re-registered */
 
     mca_base_var_groups_timestamp++;