Developers --

Ralph and I talked about this one the other day; he's trying to plug some 
memory leaks during finalize.  

Before this commit, there was a counter that counts how many times opal_init*() 
is invoked.  opal_finalize() decremented the counter, but didn't actually do 
anything else until the counter went to zero.  The original intent was that we 
should call finalize exactly as many times as we call initialize, but it 
appears that we didn't do that (e.g., we call opal_init_util() and then 
opal_init(), but then only call opal_finalize() once).

Does anyone have a reason NOT to convert the init/finalize counter to a simple 
boolean?  (i.e., can you provide a reason to back this change out?)



On Jul 8, 2011, at 2:43 AM, r...@osl.iu.edu wrote:

> Author: rhc
> Date: 2011-07-08 02:43:19 EDT (Fri, 08 Jul 2011)
> New Revision: 24862
> URL: https://svn.open-mpi.org/trac/ompi/changeset/24862
> 
> Log:
> Replace a useless counter with a boolean check to see if we have already 
> passed thru opal_finalize so we don't call finalize, and then don't pass thru 
> it (as was happening on several tools)
> 
> Text files modified: 
>   trunk/opal/runtime/opal_finalize.c |    16 ++++++----------                 
>        
>   trunk/opal/runtime/opal_init.c     |    16 ++++++----------                 
>        
>   trunk/orte/tools/orterun/orterun.c |     4 ----                             
>        
>   3 files changed, 12 insertions(+), 24 deletions(-)
> 
> Modified: trunk/opal/runtime/opal_finalize.c
> ==============================================================================
> --- trunk/opal/runtime/opal_finalize.c        (original)
> +++ trunk/opal/runtime/opal_finalize.c        2011-07-08 02:43:19 EDT (Fri, 
> 08 Jul 2011)
> @@ -54,18 +54,16 @@
> #include "opal/runtime/opal_cr.h"
> #include "opal/mca/crs/base/base.h"
> 
> -extern int opal_initialized;
> -extern int opal_util_initialized;
> +extern bool opal_initialized;
> +extern bool opal_util_initialized;
> 
> int
> opal_finalize_util(void)
> {
> -    if( --opal_util_initialized != 0 ) {
> -        if( opal_util_initialized < 0 ) {
> -            return OPAL_ERROR;
> -        }
> +    if (!opal_util_initialized) {
>         return OPAL_SUCCESS;
>     }
> +    opal_util_initialized = false;
> 
>     /* Clear out all the registered MCA params */
>     mca_base_param_finalize();
> @@ -111,12 +109,10 @@
> int
> opal_finalize(void)
> {
> -    if( --opal_initialized != 0 ) {
> -        if( opal_initialized < 0 ) {
> -            return OPAL_ERROR;
> -        }
> +    if (!opal_initialized) {
>         return OPAL_SUCCESS;
>     }
> +    opal_initialized = false;
> 
>     /* close the checkpoint and restart service */
>     opal_cr_finalize();
> 
> Modified: trunk/opal/runtime/opal_init.c
> ==============================================================================
> --- trunk/opal/runtime/opal_init.c    (original)
> +++ trunk/opal/runtime/opal_init.c    2011-07-08 02:43:19 EDT (Fri, 08 Jul 
> 2011)
> @@ -68,8 +68,8 @@
> #endif
> const char opal_version_string[] = OPAL_IDENT_STRING;
> 
> -int opal_initialized = 0;
> -int opal_util_initialized = 0;
> +bool opal_initialized = false;
> +bool opal_util_initialized = false;
> int opal_cache_line_size;
> 
> static int
> @@ -219,12 +219,10 @@
>     int ret;
>     char *error = NULL;
> 
> -    if( ++opal_util_initialized != 1 ) {
> -        if( opal_util_initialized < 1 ) {
> -            return OPAL_ERROR;
> -        }
> +    if (opal_util_initialized) {
>         return OPAL_SUCCESS;
>     }
> +    opal_util_initialized = true;
> 
>     /* JMS See note in runtime/opal.h -- this is temporary; to be
>        replaced with real hwloc information soon (in trunk/v1.5 and
> @@ -324,12 +322,10 @@
>     int ret;
>     char *error = NULL;
> 
> -    if( ++opal_initialized != 1 ) {
> -        if( opal_initialized < 1 ) {
> -            return OPAL_ERROR;
> -        }
> +    if (opal_initialized) {
>         return OPAL_SUCCESS;
>     }
> +    opal_initialized = true;
> 
>     /* initialize util code */
>     if (OPAL_SUCCESS != (ret = opal_init_util(pargc, pargv))) {
> 
> Modified: trunk/orte/tools/orterun/orterun.c
> ==============================================================================
> --- trunk/orte/tools/orterun/orterun.c        (original)
> +++ trunk/orte/tools/orterun/orterun.c        2011-07-08 02:43:19 EDT (Fri, 
> 08 Jul 2011)
> @@ -618,10 +618,6 @@
>         ORTE_ERROR_LOG(rc);
>         return rc;
>     }
> -    /* finalize the OPAL utils. As they are opened again from 
> orte_init->opal_init
> -     * we continue to have a reference count on them. So we have to finalize 
> them twice...
> -     */
> -    opal_finalize_util();
> 
>     /* check for request to report uri */
>     if (NULL != orterun_globals.report_uri) {
> _______________________________________________
> svn-full mailing list
> svn-f...@open-mpi.org
> http://www.open-mpi.org/mailman/listinfo.cgi/svn-full


-- 
Jeff Squyres
jsquy...@cisco.com
For corporate legal information go to:
http://www.cisco.com/web/about/doing_business/legal/cri/


Reply via email to