CSE320/course_tools/boxfort-commit-ac0507b

From ac0507b3f45fe58100b528baeb8ca04270b4a8ff Mon Sep 17 00:00:00 2001
From: "Franklin \"Snaipe\" Mathieu" <me@snai.pe>
Date: Mon, 23 Mar 2020 05:52:23 +0000
Subject: timeout-posix: fix race condition

The posix timeout code was racy -- if a timeout was created, and
cancelled before the watchdog had any chance to run (because the worker
would exit too quickly, or because the thread would not be scheduled
quickly enough). This, in turn, made the watchdog wait forever for the
timeout queue to be nonempty.

This fixes the race by preventing the watchdog from ever waiting for the
queue to fill up -- it's actually not possible for the queue to be
empty during initialization, because the watchdog thread will be made to
wait for the initialization lock to be released. This means that the
only time where the queue is empty is when the watchdog has been
started, but the worker already exited/the timeout was cancelled.

In addition, this fix simplifies slightly the way that the watchdog is
collected -- we no longer try to join the thread, but we make it
detached from the get go.

This addresses Snaipe/Criterion#345.

diff --git a/src/timeout-posix.c b/src/timeout-posix.c
index 53bd181..2e9a210 100644
--- a/src/timeout-posix.c
+++ b/src/timeout-posix.c
@@ -22,13 +22,13 @@
  * THE SOFTWARE.
  */
 #include <assert.h>
+#include <errno.h>
 #include <pthread.h>
-#include <time.h>
+#include <signal.h>
 #include <stdint.h>
 #include <stdlib.h>
-#include <errno.h>
-#include <signal.h>
 #include <string.h>
+#include <time.h>
 
 #include "config.h"
 #include "sandbox.h"
@@ -48,11 +48,9 @@ static struct {
     int thread_active;
     pthread_mutex_t sync;
     pthread_cond_t cond;
-    pthread_cond_t termcond;
 } self = {
     .sync = PTHREAD_MUTEX_INITIALIZER,
     .cond = PTHREAD_COND_INITIALIZER,
-    .termcond = PTHREAD_COND_INITIALIZER,
 };
 
 static int timespec_cmp(struct timespec *a, struct timespec *b)
@@ -96,8 +94,6 @@ static void to_timespec(double timeout, struct timespec *timeo)
 static void *timeout_killer_fn(void *nil)
 {
     pthread_mutex_lock(&self.sync);
-    while (!self.requests)
-        pthread_cond_wait(&self.cond, &self.sync);
 
     struct bxfi_timeout_request *req;
     for (;;) {
@@ -125,7 +121,7 @@ static void *timeout_killer_fn(void *nil)
         free(req);
     }
 end:
-    pthread_cond_broadcast(&self.termcond);
+    self.thread_active = 0;
     pthread_mutex_unlock(&self.sync);
     return nil;
 }
@@ -137,10 +133,6 @@ void bxfi_reset_timeout_killer(void)
 
     memcpy(&self.sync, &mutex, sizeof (mutex));
     memcpy(&self.cond, &cond, sizeof (cond));
-    memcpy(&self.termcond, &cond, sizeof (cond));
-
-    if (self.requests)
-        pthread_join(self.thread, NULL);
 }
 
 int bxfi_push_timeout(struct bxfi_sandbox *instance, double timeout)
@@ -159,10 +151,16 @@ int bxfi_push_timeout(struct bxfi_sandbox *instance, double timeout)
 
     pthread_mutex_lock(&self.sync);
     if (!self.requests) {
-        if (self.thread_active)
-            pthread_join(self.thread, NULL);
+        pthread_attr_t attrs;
+        if ((rc = pthread_attr_init(&attrs)) == -1) {
+            rc = -errno;
+            goto error;
+        }
+        pthread_attr_setdetachstate(&attrs, PTHREAD_CREATE_DETACHED);
+
         self.thread_active = 1;
-        rc = -pthread_create(&self.thread, NULL, timeout_killer_fn, NULL);
+        rc = -pthread_create(&self.thread, &attrs, timeout_killer_fn, NULL);
+        pthread_attr_destroy(&attrs);
         if (rc)
             goto error;
     }
@@ -177,7 +175,6 @@ int bxfi_push_timeout(struct bxfi_sandbox *instance, double timeout)
     *nptr = req;
 
     pthread_cond_broadcast(&self.cond);
-    pthread_cond_broadcast(&self.termcond);
     pthread_mutex_unlock(&self.sync);
     return 0;
 
@@ -204,17 +201,6 @@ void bxfi_cancel_timeout(struct bxfi_sandbox *instance)
     }
     if (cancelled) {
         pthread_cond_broadcast(&self.cond);
-        if (!self.requests) {
-            while (self.cancelled && !self.requests)
-                pthread_cond_wait(&self.termcond, &self.sync);
-            if (self.requests)
-                goto end;
-            if (self.thread_active) {
-                pthread_join(self.thread, NULL);
-                self.thread_active = 0;
-            }
-        }
     }
-end:
     pthread_mutex_unlock(&self.sync);
 }