Hi,
On Tue, 2019-11-19 at 16:22 -0500, Frank Ch. Eigler wrote:
> On Tue, Nov 19, 2019 at 09:15:20PM +0100, Mark Wielaard wrote:
> > > That's what the doc says. What the code does, as far back as the year
> > > 2001, is have a static flag/counter in curl_global_init() that
> > > prevents multiple initialization.
> >
> > But the warning isn't about multiple initialization. It is about
> > initialization when other threads are running that might be using any
> > of the libcurl support libraries.
>
> Since 2001, the curl_global_init function does nothing to interfere
> with any libcurl activity, if the library is already initialized. Any
> call to the normal libcurl functions first calls this function. I
> guess I just fail to see a plausible problem scenario short of a
> minuscule race over incrementing an initialization counter, which is a
> race that every other libcurl user has accepted.
But it isn't just about interference with other libcurl activity. If
you look at the curl_global_init code you see that it actually calls a
lot of code in other libraries. It is the curl_global_init code that
shouldn't be run in a multi-threaded environment. That it is acceptable
to others doesn't immediately make it safe to use in our case. We are
slowly trying to make libdw.so into a multi-tread safe library and do
expect it to be used in multi-threaded code. We aren't fully there yet.
But it would be a shame to introduce more issues if we can prevent it.
> > > > That is why I think doing the dlopen of libdebuginfod.so eagerly from a
> > > > libdw.so constructor function or _init might be necessary to make sure
> > > > no other threads are running yet.
> > >
> > > That's not even enough for "sure" - if we're so deep in the
> > > hypotheticals hole, an application could be dlopen()ing libdw.so
> > > itself.
> >
> > It could, but I think that would be unlikely. We can at least document
> > that it is unsafe to use libdw.so with dlopen. But if it is done,
> > could we detect it and not do the loading of libdebuginfod.so in that
> > case?
>
> I don't know how to do that.
I assume you mean the second part. The attached is what I would propose
for the first part. Do you think that is a bad idea?
Thanks,
Mark
diff --git a/libdwfl/debuginfod-client.c b/libdwfl/debuginfod-client.c
index 37a4c71f..ee604ad9 100644
--- a/libdwfl/debuginfod-client.c
+++ b/libdwfl/debuginfod-client.c
@@ -46,32 +46,7 @@ get_client (Dwfl *dwfl)
if (dwfl->debuginfod != NULL)
return dwfl->debuginfod;
- if (fp_debuginfod_begin == NULL)
- {
- void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
-
- if (debuginfod_so == NULL)
- debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
-
- if (debuginfod_so != NULL)
- {
- fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
- fp_debuginfod_find_executable = dlsym (debuginfod_so,
- "debuginfod_find_executable");
- fp_debuginfod_find_debuginfo = dlsym (debuginfod_so,
- "debuginfod_find_debuginfo");
- fp_debuginfod_end = dlsym (debuginfod_so, "debuginfod_end");
- }
-
- if (fp_debuginfod_begin == NULL
- || fp_debuginfod_find_executable == NULL
- || fp_debuginfod_find_debuginfo == NULL
- || fp_debuginfod_end == NULL)
- fp_debuginfod_begin = (void *) -1; /* never try again */
- }
-
- if (fp_debuginfod_begin != NULL
- && fp_debuginfod_begin != (void *) -1)
+ if (fp_debuginfod_begin != NULL)
{
dwfl->debuginfod = (*fp_debuginfod_begin) ();
return dwfl->debuginfod;
@@ -120,3 +95,37 @@ __libdwfl_debuginfod_end (debuginfod_client *c)
if (c != NULL)
(*fp_debuginfod_end) (c);
}
+
+/* Try to get the libdebuginfod library functions to make sure
+ everything is initialized early. */
+void __attribute__ ((constructor))
+__libdwfl_debuginfod_init (void)
+{
+ void *debuginfod_so = dlopen("libdebuginfod-" VERSION ".so", RTLD_LAZY);
+
+ if (debuginfod_so == NULL)
+ debuginfod_so = dlopen("libdebuginfod.so", RTLD_LAZY);
+
+ if (debuginfod_so != NULL)
+ {
+ fp_debuginfod_begin = dlsym (debuginfod_so, "debuginfod_begin");
+ fp_debuginfod_find_executable = dlsym (debuginfod_so,
+ "debuginfod_find_executable");
+ fp_debuginfod_find_debuginfo = dlsym (debuginfod_so,
+ "debuginfod_find_debuginfo");
+ fp_debuginfod_end = dlsym (debuginfod_so, "debuginfod_end");
+
+ /* We either get them all, or we get none. */
+ if (fp_debuginfod_begin == NULL
+ || fp_debuginfod_find_executable == NULL
+ || fp_debuginfod_find_debuginfo == NULL
+ || fp_debuginfod_end == NULL)
+ {
+ fp_debuginfod_begin = NULL;
+ fp_debuginfod_find_executable = NULL;
+ fp_debuginfod_find_debuginfo = NULL;
+ fp_debuginfod_end = NULL;
+ dlclose (debuginfod_so);
+ }
+ }
+}
