Add pthread cleanup to listener (to cleanup mutex)

[ust.git] / libust / tracectl.c

diff --git a/libust/tracectl.c b/libust/tracectl.c
index d32deba978ac8d2500fa6a222d07a520a2db2ebc..b058cb3ce19d0d317790ddbf03103164508d1eb4 100644 (file)
--- a/libust/tracectl.c
+++ b/libust/tracectl.c
@@ -63,6 +63,16 @@ static char receive_buffer[USTCOMM_BUFFER_SIZE];
 static char send_buffer[USTCOMM_BUFFER_SIZE];
 
 static int epoll_fd;
+
+/*
+ * Listener thread data vs fork() protection mechanism. Ensures that no listener
+ * thread mutexes and data structures are being concurrently modified or held by
+ * other threads when fork() is executed.
+ */
+static pthread_mutex_t listener_thread_data_mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/* Mutex protecting listen_sock. Nests inside listener_thread_data_mutex. */
+static pthread_mutex_t listen_sock_mutex = PTHREAD_MUTEX_INITIALIZER;
 static struct ustcomm_sock *listen_sock;
 
 extern struct chan_info_struct chan_infos[];
@@ -533,9 +543,19 @@ unlock_traces:
        return retval;
 }
 
+static void release_listener_mutex(void *ptr)
+{
+       pthread_mutex_unlock(&listener_thread_data_mutex);
+}
+
 static void listener_cleanup(void *ptr)
 {
-       ustcomm_del_named_sock(listen_sock, 0);
+       pthread_mutex_lock(&listen_sock_mutex);
+       if (listen_sock) {
+               ustcomm_del_named_sock(listen_sock, 0);
+               listen_sock = NULL;
+       }
+       pthread_mutex_unlock(&listen_sock_mutex);
 }
 
 static void force_subbuf_switch()
@@ -1080,6 +1100,8 @@ void *listener_main(void *p)
                }
 
                for (i = 0; i < nfds; i++) {
+                       pthread_mutex_lock(&listener_thread_data_mutex);
+                       pthread_cleanup_push(release_listener_mutex, NULL);
                        epoll_sock = (struct ustcomm_sock *)events[i].data.ptr;
                        if (epoll_sock == listen_sock) {
                                addr_size = sizeof(struct sockaddr);
@@ -1108,6 +1130,7 @@ void *listener_main(void *p)
                                                           epoll_sock->fd);
                                }
                        }
+                       pthread_cleanup_pop(1); /* release listener mutex */
                }
        }
 
@@ -1569,6 +1592,7 @@ static void ust_fork(void)
 {
        struct ust_buffer *buf, *buf_tmp;
        struct ustcomm_sock *sock, *sock_tmp;
+       struct ust_trace *trace, *trace_tmp;
        int result;
 
        /* FIXME: technically, the locks could have been taken before the fork */
@@ -1577,11 +1601,14 @@ static void ust_fork(void)
        /* Get the pid of the new process */
        processpid = getpid();
 
-       /* break lock if necessary */
-       ltt_unlock_traces();
+       /*
+        * FIXME: This could be prettier, we loop over the list twice and
+        * following good locking practice should lock around the loop
+        */
+       cds_list_for_each_entry_safe(trace, trace_tmp, &ltt_traces.head, list) {
+               ltt_trace_stop(trace->trace_name);
+       }
 
-       ltt_trace_stop("auto");
-       ltt_trace_destroy("auto", 1);
        /* Delete all active connections, but leave them in the epoll set */
        cds_list_for_each_entry_safe(sock, sock_tmp, &ust_socks, list) {
                ustcomm_del_sock(sock, 1);
@@ -1590,19 +1617,22 @@ static void ust_fork(void)
        /* Delete all blocked consumers */
        cds_list_for_each_entry_safe(buf, buf_tmp, &open_buffers_list,
                                 open_buffers_list) {
-               result = close(buf->data_ready_fd_read);
-               if (result == -1) {
-                       PERROR("close");
-               }
-               result = close(buf->data_ready_fd_write);
-               if (result == -1) {
-                       PERROR("close");
-               }
                cds_list_del(&buf->open_buffers_list);
        }
 
-       /* Clean up the listener socket and epoll, keeping the scoket file */
-       ustcomm_del_named_sock(listen_sock, 1);
+       /*
+        * FIXME: This could be prettier, we loop over the list twice and
+        * following good locking practice should lock around the loop
+        */
+       cds_list_for_each_entry_safe(trace, trace_tmp, &ltt_traces.head, list) {
+               ltt_trace_destroy(trace->trace_name, 1);
+       }
+
+       /* Clean up the listener socket and epoll, keeping the socket file */
+       if (listen_sock) {
+               ustcomm_del_named_sock(listen_sock, 1);
+               listen_sock = NULL;
+       }
        close(epoll_fd);
 
        /* Re-start the launch sequence */
@@ -1657,6 +1687,16 @@ void ust_before_fork(ust_fork_info_t *fork_info)
                 PERROR("sigprocmask");
                 return;
         }
+
+       /*
+        * Take the fork lock to make sure we are not in the middle of
+        * something in the listener thread.
+        */
+       pthread_mutex_lock(&listener_thread_data_mutex);
+       /*
+        * Hold listen_sock_mutex to protect from listen_sock teardown.
+        */
+       pthread_mutex_lock(&listen_sock_mutex);
 }
 
 /* Don't call this function directly in a traced program */
@@ -1664,6 +1704,9 @@ static void ust_after_fork_common(ust_fork_info_t *fork_info)
 {
        int result;
 
+       pthread_mutex_unlock(&listen_sock_mutex);
+       pthread_mutex_unlock(&listener_thread_data_mutex);
+
         /* Restore signals */
         result = sigprocmask(SIG_SETMASK, &fork_info->orig_sigs, NULL);
         if (result == -1) {
@@ -1674,7 +1717,7 @@ static void ust_after_fork_common(ust_fork_info_t *fork_info)
 
 void ust_after_fork_parent(ust_fork_info_t *fork_info)
 {
-       /* Reenable signals */
+       /* Release mutexes and reenable signals */
        ust_after_fork_common(fork_info);
 }
 
@@ -1683,7 +1726,15 @@ void ust_after_fork_child(ust_fork_info_t *fork_info)
        /* First sanitize the child */
        ust_fork();
 
-       /* Then reenable interrupts */
+       /* Then release mutexes and reenable signals */
        ust_after_fork_common(fork_info);
+
+       /*
+        * Make sure we clean up the urcu-bp thread list in the child by running
+        * the garbage collection before any pthread_create can be called.
+        * Failure to do so could lead to a deadlock caused by reuse of a thread
+        * ID before urcu-bp garbage collection is performed.
+        */
+       synchronize_rcu();
 }