Re: ltdl bugs

[Jeff Squyres]

On Apr 3, 2005, at 1:52 AM, Ralf Wildenhues wrote:

> > > Hate to reply to myself, but I would consider that the memory not 
> > > being
> > > cleaned up by lt_dlexit() to be a bug. Darn, thought I'd get away 
> > > with
> > > the
> > > 'not a bug' response.
> >
> > 1. lt_dlclose() needs to free the memory.
>
> No.  Not a bug.  I have audited this once, thought it was a bug, then
> tried to fix it.  Found out all you need to do is think about the
> residency of the module.   (Hint: try a file != self.)
>
> If you had tried to execute the program in a loop, you'd found out that
> memory will _not_ accumulate (IIRC -- if not, please report back).
> Furthermore, if you (not possible without touching internals, I think)
> set the residency to no, the memory would go away.
>
> These 32some bytes you will just have to live with.

I have to disagree -- this is definitely a bug.  You can open 
self(NULL), and malloc is called.  Then you call close, and that memory 
is not freed.  That's a bug.  I understand that you can't "unload" 
self, but you can still free the memory.

I don't think you need to change the residency of the module -- I agree 
that this doesn't make sense (you can't, and shouldn't try, to unload 
it).  But here's a relatively simple patch that seems to fix the issue 
(I did not re-indent the rest of the function in order to make the 
patch simple/clear for e-mail purposes); it's similar to the pseudocode 
that I sent before, but with correct logic and the details filled in:

-----
--- ltdl.c.orig 2005-04-03 08:49:11.000000000 -0400
+++ ltdl.c      2005-04-03 08:48:13.000000000 -0400
@@ -3798,8 +3798,22 @@
      correctly incase the user decides to reset the residency flag
      later (even though the API makes no provision for that at the
      moment).  */
-  if (handle->info.ref_count <= 0 && !LT_DLIS_RESIDENT (handle))
-    {
+  if (handle->info.ref_count <= 0) {
+      if (LT_DLIS_RESIDENT (handle))
+        {
+            if (handle != handles)
+                {
+                    last->next = handle->next;
+                }
+            else
+                {
+                    handles = handle->next;
+                }
+            LT_DLFREE (handle);
+            goto done;
+        }
+      else
+        {
       lt_user_data data = handle->loader->dlloader_data;

       if (handle != handles)
@@ -3823,6 +3837,7 @@

       goto done;
     }
+  }

   if (LT_DLIS_RESIDENT (handle))
     {
-----

Specifically, it says that if the module is resident, then remove it 
from the "handles" list and then free the handle.  That's all you need 
to do, right?

I know I'm harping on this, but it's not the fact that it's only 32 
bytes; it's the fact that this and some GNU flex issues are the only 
non-operating-system memory leaks that are currently showing up in Open 
MPI.  :-\

--
{+} Jeff Squyres
{+} address@hidden
{+} http://www.lam-mpi.org/