Fix: fd tracker: provide async-signal-safety for close wrapper

close(3) is part of the async-signal-safe functions. Therefore, it is
expected that the close wrapper provided by liblttng-ust-fd-tracker
behaves in a async-signal-safe way.

Use a similar strategy as ust_lock() does: disable signals when taking
and releasing the lock, and keep track of nesting with a TLS variable.
This ensures signals are restored to their original state when close(3)
ends up being invoked.

Fixes: #1199
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/liblttng-ust-comm/lttng-ust-fd-tracker.c b/liblttng-ust-comm/lttng-ust-fd-tracker.c
index 1118163a201af456ac6ed28d490e09f458af08ef..9659f349e54ac85a3afad50b2cfc2055abcfacfc 100644 (file)
--- a/liblttng-ust-comm/lttng-ust-fd-tracker.c
+++ b/liblttng-ust-comm/lttng-ust-fd-tracker.c
@@ -32,6 +32,8 @@
 #include <sys/time.h>
 #include <fcntl.h>
 #include <pthread.h>
+#include <signal.h>
+#include <stdbool.h>
 #include <urcu/compiler.h>
 #include <urcu/tls-compat.h>
 #include <urcu/system.h>
@@ -75,9 +77,11 @@ static int ust_safe_guard_saved_cancelstate;
 
 /*
  * Track whether we are within lttng-ust or application, for close
- * system call override by LD_PRELOAD library.
+ * system call override by LD_PRELOAD library. This also tracks whether
+ * we are invoking close() from a signal handler nested on an
+ * application thread.
  */
-static DEFINE_URCU_TLS(int, thread_fd_tracking);
+static DEFINE_URCU_TLS(int, ust_fd_mutex_nest);
 
 /* fd_set used to book keep fd being used by lttng-ust. */
 static fd_set *lttng_fd_set;
@@ -90,7 +94,7 @@ static int init_done;
  */
 void lttng_ust_fixup_fd_tracker_tls(void)
 {
-       asm volatile ("" : : "m" (URCU_TLS(thread_fd_tracking)));
+       asm volatile ("" : : "m" (URCU_TLS(ust_fd_mutex_nest)));
 }
 
 /*
@@ -134,37 +138,63 @@ void lttng_ust_init_fd_tracker(void)
 
 void lttng_ust_lock_fd_tracker(void)
 {
+       sigset_t sig_all_blocked, orig_mask;
        int ret, oldstate;
 
        ret = pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &oldstate);
        if (ret) {
                ERR("pthread_setcancelstate: %s", strerror(ret));
        }
-       URCU_TLS(thread_fd_tracking) = 1;
-       /*
-        * Ensure the compiler don't move the store after the close()
-        * call in case close() would be marked as leaf.
-        */
-       cmm_barrier();
-       pthread_mutex_lock(&ust_safe_guard_fd_mutex);
-       ust_safe_guard_saved_cancelstate = oldstate;
+       sigfillset(&sig_all_blocked);
+       ret = pthread_sigmask(SIG_SETMASK, &sig_all_blocked, &orig_mask);
+       if (ret) {
+               ERR("pthread_sigmask: %s", strerror(ret));
+       }
+       if (!URCU_TLS(ust_fd_mutex_nest)++) {
+               /*
+                * Ensure the compiler don't move the store after the close()
+                * call in case close() would be marked as leaf.
+                */
+               cmm_barrier();
+               pthread_mutex_lock(&ust_safe_guard_fd_mutex);
+               ust_safe_guard_saved_cancelstate = oldstate;
+       }
+       ret = pthread_sigmask(SIG_SETMASK, &orig_mask, NULL);
+       if (ret) {
+               ERR("pthread_sigmask: %s", strerror(ret));
+       }
 }
 
 void lttng_ust_unlock_fd_tracker(void)
 {
+       sigset_t sig_all_blocked, orig_mask;
        int ret, newstate, oldstate;
+       bool restore_cancel = false;
 
-       newstate = ust_safe_guard_saved_cancelstate;
-       pthread_mutex_unlock(&ust_safe_guard_fd_mutex);
+       sigfillset(&sig_all_blocked);
+       ret = pthread_sigmask(SIG_SETMASK, &sig_all_blocked, &orig_mask);
+       if (ret) {
+               ERR("pthread_sigmask: %s", strerror(ret));
+       }
        /*
         * Ensure the compiler don't move the store before the close()
         * call, in case close() would be marked as leaf.
         */
        cmm_barrier();
-       URCU_TLS(thread_fd_tracking) = 0;
-       ret = pthread_setcancelstate(newstate, &oldstate);
+       if (!--URCU_TLS(ust_fd_mutex_nest)) {
+               newstate = ust_safe_guard_saved_cancelstate;
+               restore_cancel = true;
+               pthread_mutex_unlock(&ust_safe_guard_fd_mutex);
+       }
+       ret = pthread_sigmask(SIG_SETMASK, &orig_mask, NULL);
        if (ret) {
-               ERR("pthread_setcancelstate: %s", strerror(ret));
+               ERR("pthread_sigmask: %s", strerror(ret));
+       }
+       if (restore_cancel) {
+               ret = pthread_setcancelstate(newstate, &oldstate);
+               if (ret) {
+                       ERR("pthread_setcancelstate: %s", strerror(ret));
+               }
        }
 }
 
@@ -255,7 +285,7 @@ int lttng_ust_add_fd_to_tracker(int fd)
         * constructors.
         */
        lttng_ust_init_fd_tracker();
-       assert(URCU_TLS(thread_fd_tracking));
+       assert(URCU_TLS(ust_fd_mutex_nest));
 
        if (IS_FD_STD(fd)) {
                ret = dup_std_fd(fd);
@@ -288,7 +318,7 @@ void lttng_ust_delete_fd_from_tracker(int fd)
         */
        lttng_ust_init_fd_tracker();
 
-       assert(URCU_TLS(thread_fd_tracking));
+       assert(URCU_TLS(ust_fd_mutex_nest));
        /* Not a valid fd. */
        assert(IS_FD_VALID(fd));
        /* Deleting an fd which was not set. */
@@ -318,7 +348,7 @@ int lttng_ust_safe_close_fd(int fd, int (*close_cb)(int fd))
         * If called from lttng-ust, we directly call close without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(thread_fd_tracking))
+       if (URCU_TLS(ust_fd_mutex_nest))
                return close_cb(fd);
 
        lttng_ust_lock_fd_tracker();
@@ -354,7 +384,7 @@ int lttng_ust_safe_fclose_stream(FILE *stream, int (*fclose_cb)(FILE *stream))
         * If called from lttng-ust, we directly call fclose without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(thread_fd_tracking))
+       if (URCU_TLS(ust_fd_mutex_nest))
                return fclose_cb(stream);
 
        fd = fileno(stream);
@@ -417,7 +447,7 @@ int lttng_ust_safe_closefrom_fd(int lowfd, int (*close_cb)(int fd))
         * If called from lttng-ust, we directly call close without
         * validating whether the FD is part of the tracked set.
         */
-       if (URCU_TLS(thread_fd_tracking)) {
+       if (URCU_TLS(ust_fd_mutex_nest)) {
                for (i = lowfd; i < lttng_ust_max_fd; i++) {
                        if (close_cb(i) < 0) {
                                switch (errno) {