Add missing listener threads data vs fork() protection

The following races are problematic:

- fork() occurs concurrently with listener thread receiving commands.
  - Mutexes and data structures can be left in incoherent state.
- fork() occurs concurrently with ust library destructor.
  - listen_sock can be left in incoherent state in the child.

Protect these resources with their own specific mutex.
listener_thread_data_mutex protects all data/mutexes touched by the listener
thread. It is also held across fork to make sure the child see a coherent
version of these structures.

listen_sock_mutex protects the listen_sock teardown (pthread cancel done at
libust destructor). Is is also held across fork() to protect from concurrent
teardown of listen_sock. We add a check around listen_sock teardown to see if it
has already been deleted (which could happen if the destructor runs concurrently
with fork().

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

diff --git a/libust/tracectl.c b/libust/tracectl.c
index f720244530c9f60b77f5978875f712a5ea69658a..bef4537dd4ffeb5311c4e83dabbf60adfbb3fbd3 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[];
@@ -535,7 +545,12 @@ unlock_traces:
 
 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 +1095,7 @@ void *listener_main(void *p)
                }
 
                for (i = 0; i < nfds; i++) {
+                       pthread_mutex_lock(&listener_thread_data_mutex);
                        epoll_sock = (struct ustcomm_sock *)events[i].data.ptr;
                        if (epoll_sock == listen_sock) {
                                addr_size = sizeof(struct sockaddr);
@@ -1108,6 +1124,7 @@ void *listener_main(void *p)
                                                           epoll_sock->fd);
                                }
                        }
+                       pthread_mutex_unlock(&listener_thread_data_mutex);
                }
        }
 
@@ -1578,9 +1595,6 @@ 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
@@ -1608,8 +1622,11 @@ static void ust_fork(void)
                ltt_trace_destroy(trace->trace_name, 1);
        }
 
-       /* Clean up the listener socket and epoll, keeping the scoket file */
-       ustcomm_del_named_sock(listen_sock, 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 */
@@ -1664,6 +1681,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 */
@@ -1671,6 +1698,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) {