After discussing this further, we have decided to not merge this patch since it would prevent us from using dlopen() to load lttng-ust in an application. While the dlopen() should succeed, there would be no guarantee that the tracing is active/ready when it returns.
At this point, it seems that the appropriate fix requires a non-trivial refactor of lttng-ust's registration and initialization mechanism and threading model. Jérémie On Sat, Nov 15, 2014 at 2:18 PM, Mathieu Desnoyers <[email protected]> wrote: > Can you extend the changelog by any chance, for instance by > explaining why we need this ? Something along the lines of > fixing deadlocks involving the glibc internal dynamic loader > lock would be good. > > Thanks, > > Mathieu > > ----- Original Message ----- >> From: "Jérémie Galarneau" <[email protected]> >> To: [email protected] >> Cc: "mathieu desnoyers" <[email protected]>, "Jérémie >> Galarneau" <[email protected]> >> Sent: Saturday, November 15, 2014 8:12:38 PM >> Subject: [PATCH lttng-ust] Perform the sem_wait on the constructor semaphore >> just before the main >> >> Signed-off-by: Jérémie Galarneau <[email protected]> >> --- >> liblttng-ust/Makefile.am | 7 ++++ >> liblttng-ust/lttng-ust-comm.c | 75 >> ++++++++++++++++++++++++++++--------------- >> 2 files changed, 56 insertions(+), 26 deletions(-) >> >> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am >> index e2e1baa..802d32d 100644 >> --- a/liblttng-ust/Makefile.am >> +++ b/liblttng-ust/Makefile.am >> @@ -83,4 +83,11 @@ liblttng_ust_la_LIBADD = \ >> liblttng-ust-tracepoint.la \ >> liblttng-ust-runtime.la liblttng-ust-support.la >> >> +if LTTNG_UST_BUILD_WITH_LIBDL >> +liblttng_ust_la_LIBADD += -ldl >> +endif >> +if LTTNG_UST_BUILD_WITH_LIBC_DL >> +liblttng_ust_la_LIBADD += -lc >> +endif >> + >> liblttng_ust_la_CFLAGS = -DUST_COMPONENT="liblttng_ust" -fno-strict-aliasing >> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c >> index 3df2adc..8258e85 100644 >> --- a/liblttng-ust/lttng-ust-comm.c >> +++ b/liblttng-ust/lttng-ust-comm.c >> @@ -20,6 +20,7 @@ >> */ >> >> #define _LGPL_SOURCE >> +#define _GNU_SOURCE >> #include <sys/types.h> >> #include <sys/socket.h> >> #include <sys/mman.h> >> @@ -52,6 +53,7 @@ >> #include "compat.h" >> #include "../libringbuffer/tlsfixup.h" >> #include "lttng-ust-baddr.h" >> +#include <lttng/ust-dlfcn.h> >> >> /* >> * Has lttng ust comm constructor been called ? >> @@ -1405,10 +1407,8 @@ void lttng_ust_malloc_wrapper_init(void) >> */ >> void __attribute__((constructor)) lttng_ust_init(void) >> { >> - struct timespec constructor_timeout; >> sigset_t sig_all_blocked, orig_parent_mask; >> pthread_attr_t thread_attr; >> - int timeout_mode; >> int ret; >> >> if (uatomic_xchg(&initialized, 1) == 1) >> @@ -1446,8 +1446,6 @@ void __attribute__((constructor)) lttng_ust_init(void) >> */ >> lttng_ust_malloc_wrapper_init(); >> >> - timeout_mode = get_constructor_timeout(&constructor_timeout); >> - >> ret = sem_init(&constructor_wait, 0, 0); >> assert(!ret); >> >> @@ -1507,28 +1505,6 @@ void __attribute__((constructor)) lttng_ust_init(void) >> if (ret) { >> ERR("pthread_sigmask: %s", strerror(ret)); >> } >> - >> - switch (timeout_mode) { >> - case 1: /* timeout wait */ >> - do { >> - ret = sem_timedwait(&constructor_wait, >> - &constructor_timeout); >> - } while (ret < 0 && errno == EINTR); >> - if (ret < 0 && errno == ETIMEDOUT) { >> - ERR("Timed out waiting for lttng-sessiond"); >> - } else { >> - assert(!ret); >> - } >> - break; >> - case -1:/* wait forever */ >> - do { >> - ret = sem_wait(&constructor_wait); >> - } while (ret < 0 && errno == EINTR); >> - assert(!ret); >> - break; >> - case 0: /* no timeout */ >> - break; >> - } >> } >> >> static >> @@ -1703,3 +1679,50 @@ void lttng_ust_sockinfo_session_enabled(void *owner) >> struct sock_info *sock_info = owner; >> sock_info->statedump_pending = 1; >> } >> + >> +int __libc_start_main(int (*main) (int, char **, char **), int argc, >> + char **ubp_av, void (*init) (void), void (*fini) (void), >> + void (*rtld_fini) (void), void (*stack_end)) >> +{ >> + int ret; >> + int timeout_mode; >> + struct timespec constructor_timeout; >> + int (*libc_main_func)(int (*main) (int, char **, char **), >> + int argc, char **ubp_av, void (*init) (void), >> + void (*fini) (void), void (*rtld_fini) (void), >> + void (*stack_end)); >> + >> + /* Retrieve the original __libc_start_main function */ >> + libc_main_func = dlsym(RTLD_NEXT, "__libc_start_main"); >> + if (libc_main_func == NULL) { >> + ERR("unable to find \"__libc_start_main\" symbol\n"); >> + errno = ENOSYS; >> + return -1; >> + } >> + >> + timeout_mode = get_constructor_timeout(&constructor_timeout); >> + switch (timeout_mode) { >> + case 1: /* timeout wait */ >> + do { >> + ret = sem_timedwait(&constructor_wait, >> + &constructor_timeout); >> + } while (ret < 0 && errno == EINTR); >> + if (ret < 0 && errno == ETIMEDOUT) { >> + ERR("Timed out waiting for lttng-sessiond"); >> + } else { >> + assert(!ret); >> + } >> + break; >> + case -1:/* wait forever */ >> + do { >> + ret = sem_wait(&constructor_wait); >> + } while (ret < 0 && errno == EINTR); >> + assert(!ret); >> + break; >> + case 0: /* no timeout */ >> + break; >> + } >> + >> + /* Call the original function */ >> + return libc_main_func(main, argc, ubp_av, init, fini, rtld_fini, >> stack_end); >> +} >> -- >> 2.1.3 >> >> > > -- > Mathieu Desnoyers > EfficiOS Inc. > http://www.efficios.com -- Jérémie Galarneau EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
