Re: HEAD: fix lt_dlloader_remove memleak in libltdl

[Ralf Wildenhues]

* Eric Blake wrote on Sat, Sep 01, 2007 at 02:33:19PM CEST:
> According to Ralf Wildenhues on 9/1/2007 4:46 AM:
> > --- libltdl/lt_dlloader.c   4 Jul 2007 23:05:04 -0000       1.11
> > +++ libltdl/lt_dlloader.c   1 Sep 2007 10:02:00 -0000
> > @@ -142,17 +142,28 @@
> >    return (const lt_dlvtable *) (loader ? ((SList *) loader)->userdata : 0);
> >  }
> >  
> > +/* Callback function to iterate over all handles */
> > +static int
> > +find_all_handles (lt_dlhandle LT__UNUSED handle, const char * LT__UNUSED 
> > id)
> > +{
> > +  return 0;
> > +}
> 
> Either we should expose this function, or it is not necessary.  I
> remembered that at one point, we had to fix loaders/loadlibrary.c to do
> something similar in iterating over all modules, but looking closer,
> get_vtable registers with a NULL callback.  In fact, lt_dlhandle_iterate
> special cases a NULL filter callback to visit everything.  But the
> documentation in libtool.texi does not mention that.

Thanks.  Updated patch below.  OK to apply?

Cheers,
Ralf

2007-09-01  Ralf Wildenhues  <address@hidden>

        * doc/libtool.texi (User defined module data)
        <lt_dlinterface_register>: Document that a NULL place matches
        all modules.
        * libltdl/lt_dlloader.c (lt_dlloader_remove): Actually iterate
        over all open modules when looking for modules that use it.
        If a resident module is found, return but do not set the error
        string.
        * libltdl/ltdl.c (lt_dlexit): When removing dlloaders, ignore
        errors that stem from earlier failed commands.  Exposed by
        lt_dladvise test.
        Fixes regression over branch-1-5.
        Memleak report as Coverity CID 19 via Jeff Squyres.

Index: doc/libtool.texi
===================================================================
RCS file: /cvsroot/libtool/libtool/doc/libtool.texi,v
retrieving revision 1.230
diff -u -r1.230 libtool.texi
--- doc/libtool.texi    23 Jul 2007 22:35:29 -0000      1.230
+++ doc/libtool.texi    1 Sep 2007 13:22:44 -0000
@@ -4022,7 +4022,8 @@
 and in return obtain a unique key to store and retrieve per-module data.
 You supply an @var{id_string} and @var{iface} so that the resulting
 @code{lt_dlinterface_id} can be used to filter the module handles
-returned by the iteration functions below.
+returned by the iteration functions below.  If @var{iface} is @code{NULL},
+all modules will be matched.
 @end deftypefun
 
 @deftypefun void lt_dlinterface_free (@w{lt_dlinterface_id @var{iface}})
Index: libltdl/lt_dlloader.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/lt_dlloader.c,v
retrieving revision 1.11
diff -u -r1.11 lt_dlloader.c
--- libltdl/lt_dlloader.c       4 Jul 2007 23:05:04 -0000       1.11
+++ libltdl/lt_dlloader.c       1 Sep 2007 13:20:18 -0000
@@ -146,13 +146,18 @@
 /* Return the contents of the first item in the global loader list
    with a matching NAME after removing it from that list.  If there
    was no match, return NULL; if there is an error, return NULL and
-   set an error for lt_dlerror; in either case, the loader list is
-   not changed if NULL is returned.  */
+   set an error for lt_dlerror; do not set an error if only resident
+   modules need this loader; in either case, the loader list is not
+   changed if NULL is returned.  */
 lt_dlvtable *
 lt_dlloader_remove (char *name)
 {
   const lt_dlvtable *  vtable  = lt_dlloader_find (name);
-  lt__handle *         handle  = 0;
+  static const char    id_string[] = "lt_dlloader_remove";
+  lt_dlinterface_id    iface;
+  lt_dlhandle          handle = 0;
+  int                  in_use = 0;
+  int                  in_use_by_resident = 0;
 
   if (!vtable)
     {
@@ -161,14 +166,24 @@
     }
 
   /* Fail if there are any open modules which use this loader.  */
-  for  (handle = 0; handle; handle = handle->next)
+  iface = lt_dlinterface_register (id_string, NULL);
+  while ((handle = lt_dlhandle_iterate (iface, handle)))
     {
-      if (handle->vtable == vtable)
+      lt__handle *cur = (lt__handle *) handle;
+      if (cur->vtable == vtable)
        {
-         LT__SETERROR (REMOVE_LOADER);
-         return 0;
+         in_use = 1;
+         if (lt_dlisresident (handle))
+           in_use_by_resident = 1;
        }
     }
+  lt_dlinterface_free (iface);
+  if (in_use)
+    {
+      if (!in_use_by_resident)
+       LT__SETERROR (REMOVE_LOADER);
+      return 0;
+    }
 
   /* Call the loader finalisation function.  */
   if (vtable && vtable->dlloader_exit)
Index: libltdl/ltdl.c
===================================================================
RCS file: /cvsroot/libtool/libtool/libltdl/ltdl.c,v
retrieving revision 1.263
diff -u -r1.263 ltdl.c
--- libltdl/ltdl.c      1 Sep 2007 10:49:19 -0000       1.263
+++ libltdl/ltdl.c      1 Sep 2007 13:20:19 -0000
@@ -310,6 +310,12 @@
            break;
        }
 
+      /* When removing loaders, we can only find out failure by testing
+        the error string, so avoid a spurious one from an earlier
+        failed command. */
+      if (!errors)
+       LT__SETERRORSTR (0);
+
       /* close all loaders */
       for (loader = (lt_dlloader *) lt_dlloader_next (NULL); loader;)
        {
@@ -322,7 +328,11 @@
            }
          else
            {
-             ++errors;
+             /* ignore errors due to resident modules */
+             const char *err;
+             LT__GETERROR (err);
+             if (err)
+               ++errors;
            }
 
          loader = next;