Re: [OMPI devel] RFC : what is the best way to fix the memory leak in mca/pml/bfo

[Gilles Gouaillardet]

Thanks guys !

i commited r31816 (bfo: allocate the allocator in init rather than open)
and made a CMR

based on mtt results, i will push George's commit tomorrow.
and based on Rolf recommendation, i will do the CMR by the end of the week
if everything
works fine

Gilles

Re: [OMPI devel] RFC : what is the best way to fix the memory leak in mca/pml/bfo

[Ralph Castain]

Memory isn't supposed to allocated in the "open" call, but only after the 
component is selected (and thus in the "init" call").


On May 15, 2014, at 10:57 PM, Gilles Gouaillardet 
 wrote:

> Folks,
> 
> there is a small memory leak in ompi/mca/pml/bfo/pml_bfo_component.c
> 
> in my environment, this module is not used.
> this means mca_pml_bfo_component_open() and mca_pml_bfo_component_close() are 
> invoked but
> mca_pml_bfo_component_init() and mca_pml_bfo_component_fini() are *not* 
> invoked.
> 
> mca_pml_bfo.allocator is currently allocated in mca_pml_bfo_component_open() 
> and released in mca_pml_bfo_component_fini()
> this causes a leak (at least in my environment, since 
> mca_pml_bfo_component_fini() is not invoked)
> 
> One option is to release the allocator in mca_pml_bfo_component_close()
> An other option is to allocate the allocator in mca_pml_bfo_component_init()
> 
> Which is the correct/best one ?
> 
> i attached two patches, which one (if any) should be commited ?
> 
> Thanks in advance for your insights
> 
> Gilles
> ___
> devel mailing list
> de...@open-mpi.org
> Subscription: http://www.open-mpi.org/mailman/listinfo.cgi/devel
> Link to this post: 
> http://www.open-mpi.org/community/lists/devel/2014/05/14815.php

[OMPI devel] RFC : what is the best way to fix the memory leak in mca/pml/bfo

[Gilles Gouaillardet]

Folks,

there is a small memory leak in ompi/mca/pml/bfo/pml_bfo_component.c

in my environment, this module is not used.
this means mca_pml_bfo_component_open() and mca_pml_bfo_component_close()
are invoked but
mca_pml_bfo_component_init() and mca_pml_bfo_component_fini() are *not*
invoked.

mca_pml_bfo.allocator is currently allocated in
mca_pml_bfo_component_open() and released in mca_pml_bfo_component_fini()
this causes a leak (at least in my environment, since
mca_pml_bfo_component_fini() is not invoked)

One option is to release the allocator in mca_pml_bfo_component_close()
An other option is to allocate the allocator in mca_pml_bfo_component_init()

Which is the correct/best one ?

i attached two patches, which one (if any) should be commited ?

Thanks in advance for your insights

Gilles
Index: ompi/mca/pml/bfo/pml_bfo_component.c
===
--- ompi/mca/pml/bfo/pml_bfo_component.c	(revision 31788)
+++ ompi/mca/pml/bfo/pml_bfo_component.c	(working copy)
@@ -12,6 +12,8 @@
  * All rights reserved.
  * Copyright (c) 2007-2010 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2010  Oracle and/or its affiliates.  All rights reserved.
+ * Copyright (c) 2014  Research Organization for Information Science
+ * and Technology (RIST). All rights reserved.
  * $COPYRIGHT$
  * 
  * Additional copyrights may follow
@@ -149,25 +151,9 @@
 
 static int mca_pml_bfo_component_open(void)
 {
-mca_allocator_base_component_t* allocator_component;
-
 mca_pml_bfo_output = opal_output_open(NULL);
 opal_output_set_verbosity(mca_pml_bfo_output, mca_pml_bfo_verbose);
 
-allocator_component = mca_allocator_component_lookup( mca_pml_bfo.allocator_name );
-if(NULL == allocator_component) {
-opal_output(0, "mca_pml_bfo_component_open: can't find allocator: %s\n", mca_pml_bfo.allocator_name);
-return OMPI_ERROR;
-}
-
-mca_pml_bfo.allocator = allocator_component->allocator_init(true,
-mca_pml_bfo_seg_alloc,
-mca_pml_bfo_seg_free, NULL);
-if(NULL == mca_pml_bfo.allocator) {
-opal_output(0, "mca_pml_bfo_component_open: unable to initialize allocator\n");
-return OMPI_ERROR;
-}
-
 mca_pml_bfo.enabled = false; 
 return mca_base_framework_open(_bml_base_framework, 0); 
 }
@@ -191,6 +177,8 @@
 bool enable_progress_threads,
 bool enable_mpi_threads )
 {
+mca_allocator_base_component_t* allocator_component;
+
 opal_output_verbose( 10, mca_pml_bfo_output,
  "in bfo, my priority is %d\n", mca_pml_bfo.priority);
 
@@ -200,6 +188,21 @@
 }
 *priority = mca_pml_bfo.priority;
 
+allocator_component = mca_allocator_component_lookup( mca_pml_bfo.allocator_name );
+if(NULL == allocator_component) {
+opal_output(0, "mca_pml_bfo_component_open: can't find allocator: %s\n", mca_pml_bfo.allocator_name);
+return NULL;
+}
+
+mca_pml_bfo.allocator = allocator_component->allocator_init(true,
+mca_pml_bfo_seg_alloc,
+mca_pml_bfo_seg_free, NULL);
+if(NULL == mca_pml_bfo.allocator) {
+opal_output(0, "mca_pml_bfo_component_open: unable to initialize allocator\n");
+return NULL;
+}
+
+
 if(OMPI_SUCCESS != mca_bml_base_init( enable_progress_threads, 
   enable_mpi_threads)) {
 return NULL;
Index: ompi/mca/pml/bfo/pml_bfo_component.c
===
--- ompi/mca/pml/bfo/pml_bfo_component.c	(revision 31785)
+++ ompi/mca/pml/bfo/pml_bfo_component.c	(working copy)
@@ -12,6 +12,8 @@
  * All rights reserved.
  * Copyright (c) 2007-2010 Cisco Systems, Inc.  All rights reserved.
  * Copyright (c) 2010  Oracle and/or its affiliates.  All rights reserved.
+ * Copyright (c) 2014  Research Organization for Information Science
+ * and Technology (RIST). All rights reserved.
  * $COPYRIGHT$
  * 
  * Additional copyrights may follow
@@ -180,6 +182,9 @@
 if (OMPI_SUCCESS != (rc = mca_base_framework_close(_bml_base_framework))) {
  return rc;
 }
+if(OMPI_SUCCESS != (rc = mca_pml_bfo.allocator->alc_finalize(mca_pml_bfo.allocator))) {
+return rc;
+}
 opal_output_close(mca_pml_bfo_output);
 
 return OMPI_SUCCESS;
@@ -237,10 +242,6 @@
 OBJ_DESTRUCT(_pml_bfo.rdma_frags);
 OBJ_DESTRUCT(_pml_bfo.lock);
 
-if(OMPI_SUCCESS != (rc = mca_pml_bfo.allocator->alc_finalize(mca_pml_bfo.allocator))) {
-return rc;
-}
-
 #if 0
 if (mca_pml_base_send_requests.fl_num_allocated !=