diff mbox

Fix dlclose / exit running in parallel resulting in dtor being called twice

Message ID 875ztx6in7.fsf@oldenburg2.str.redhat.com
State Committed
Headers

Commit Message

Florian Weimer Feb. 6, 2019, 3:28 p.m. UTC 
  * Florian Weimer:

> * Paul Pluzhnikov:
>
>> On Wed, Feb 6, 2019 at 1:37 AM Florian Weimer <fweimer@redhat.com> wrote:
>>>
>>> * Paul Pluzhnikov:
>>>
>>> > +  /* Clear any errors.  */
>>> > +  dlerror ();
>>> > +  return dso;
>>> > +}
>>>
>>> Why did you add the dlerror call in the success case?
>>
>> I don't remember / can't recall.
>>
>> But it seems like it should always be a no-op. Do you have a case
>> where dlopen() succeeds and dlerror() is not a no-op?
>
> I think it may matter for dlsym, where you need to look dlerror to tell
> if the symbol was NULL, or there was an actual error.  But you would
> have to clear dlerror *before* dlsym anyway, in case something else had
> called dlsym without clearing the error.

In other words, something like this.

Thanks,
Florian

support: Use dlerror to detect NULL symbols in xdlsym

2019-02-06  Florian Weimer  <fweimer@redhat.com>

	* support/xdlfcn.c (xdlopen, xdlclose): Do not call dlerror.
	(xdlsym): Use dlerror to detect a NULL symbol.

Comments

Paul Pluzhnikov Feb. 6, 2019, 3:41 p.m. UTC | #1 
On Wed, Feb 6, 2019 at 7:28 AM Florian Weimer <fweimer@redhat.com> wrote:

> > I think it may matter for dlsym, where you need to look dlerror to tell
> > if the symbol was NULL, or there was an actual error.  But you would
> > have to clear dlerror *before* dlsym anyway, in case something else had
> > called dlsym without clearing the error.
>
> In other words, something like this.

The patch looks good to me.

Thanks!
diff mbox

Patch

diff --git a/support/xdlfcn.c b/support/xdlfcn.c
index 2f2ac76003..b2e5c21134 100644
--- a/support/xdlfcn.c
+++ b/support/xdlfcn.c
@@ -28,22 +28,25 @@  xdlopen (const char *filename, int flags)
   if (dso == NULL)
     FAIL_EXIT1 ("error: dlopen: %s\n", dlerror ());
 
-  /* Clear any errors.  */
-  dlerror ();
-
   return dso;
 }
 
 void *
 xdlsym (void *handle, const char *symbol)
 {
+  /* Clear any pending errors.  */
+  dlerror ();
+
   void *sym = dlsym (handle, symbol);
 
   if (sym == NULL)
-    FAIL_EXIT1 ("error: dlsym: %s\n", dlerror ());
-
-  /* Clear any errors.  */
-  dlerror ();
+    {
+      const char *error = dlerror ();
+      if (error != NULL)
+        FAIL_EXIT1 ("error: dlsym: %s\n", error);
+      /* If there was no error, we found a NULL symbol.  Return the
+         NULL value in this case.  */
+    }
 
   return sym;
 }
@@ -53,7 +56,4 @@  xdlclose (void *handle)
 {
   if (dlclose (handle) != 0)
     FAIL_EXIT1 ("error: dlclose: %s\n", dlerror ());
-
-  /* Clear any errors.  */
-  dlerror ();
 }