Merge remote-tracking branch 'public/thread_coverage'

12 files changed, 94 insertions, 15 deletions

diff --git a/changes/workqueue_tests b/changes/workqueue_tests
new file mode 100644
index 0000000000..7c5d756f76
--- /dev/null
+++ b/changes/workqueue_tests
@@ -0,0 +1,4 @@
+  o Testing:
+    - Run more workqueue tests as part of "make check". These had previously
+      been implemented, but you needed to know special command-line options
+      to enable them.
diff --git a/src/common/compat_pthreads.c b/src/common/compat_pthreads.c
index 1d6db34225..79f5cec43a 100644
--- a/src/common/compat_pthreads.c
+++ b/src/common/compat_pthreads.c
@@ -102,11 +102,13 @@ void
 tor_mutex_init(tor_mutex_t *mutex)
 {
   if (PREDICT_UNLIKELY(!threads_initialized))
-    tor_threads_init();
+    tor_threads_init(); // LCOV_EXCL_LINE
   const int err = pthread_mutex_init(&mutex->mutex, &attr_recursive);
   if (PREDICT_UNLIKELY(err)) {
+    // LCOV_EXCL_START
     log_err(LD_GENERAL, "Error %d creating a mutex.", err);
-    tor_fragile_assert();
+    tor_assert_unreached();
+    // LCOV_EXCL_STOP
   }
 }
 
@@ -116,12 +118,14 @@ void
 tor_mutex_init_nonrecursive(tor_mutex_t *mutex)
 {
   int err;
-  if (PREDICT_UNLIKELY(!threads_initialized))
-    tor_threads_init();
+  if (!threads_initialized)
+    tor_threads_init(); // LCOV_EXCL_LINE
   err = pthread_mutex_init(&mutex->mutex, NULL);
   if (PREDICT_UNLIKELY(err)) {
+    // LCOV_EXCL_START
     log_err(LD_GENERAL, "Error %d creating a mutex.", err);
-    tor_fragile_assert();
+    tor_assert_unreached();
+    // LCOV_EXCL_STOP
   }
 }
 
@@ -133,8 +137,10 @@ tor_mutex_acquire(tor_mutex_t *m)
   tor_assert(m);
   err = pthread_mutex_lock(&m->mutex);
   if (PREDICT_UNLIKELY(err)) {
+    // LCOV_EXCL_START
     log_err(LD_GENERAL, "Error %d locking a mutex.", err);
-    tor_fragile_assert();
+    tor_assert_unreached();
+    // LCOV_EXCL_STOP
   }
 }
 /** Release the lock <b>m</b> so another thread can have it. */
@@ -145,8 +151,10 @@ tor_mutex_release(tor_mutex_t *m)
   tor_assert(m);
   err = pthread_mutex_unlock(&m->mutex);
   if (PREDICT_UNLIKELY(err)) {
+    // LCOV_EXCL_START
     log_err(LD_GENERAL, "Error %d unlocking a mutex.", err);
-    tor_fragile_assert();
+    tor_assert_unreached();
+    // LCOV_EXCL_STOP
   }
 }
 /** Clean up the mutex <b>m</b> so that it no longer uses any system
@@ -159,8 +167,10 @@ tor_mutex_uninit(tor_mutex_t *m)
   tor_assert(m);
   err = pthread_mutex_destroy(&m->mutex);
   if (PREDICT_UNLIKELY(err)) {
+    // LCOV_EXCL_START
     log_err(LD_GENERAL, "Error %d destroying a mutex.", err);
-    tor_fragile_assert();
+    tor_assert_unreached();
+    // LCOV_EXCL_STOP
   }
 }
 /** Return an integer representing this thread. */
@@ -210,8 +220,10 @@ void
 tor_cond_uninit(tor_cond_t *cond)
 {
   if (pthread_cond_destroy(&cond->cond)) {
+    // LCOV_EXCL_START
     log_warn(LD_GENERAL,"Error freeing condition: %s", strerror(errno));
     return;
+    // LCOV_EXCL_STOP
   }
 }
 /** Wait until one of the tor_cond_signal functions is called on <b>cond</b>.
@@ -232,7 +244,7 @@ tor_cond_wait(tor_cond_t *cond, tor_mutex_t *mutex, const struct timeval *tv)
         /* EINTR should be impossible according to POSIX, but POSIX, like the
          * Pirate's Code, is apparently treated "more like what you'd call
          * guidelines than actual rules." */
-        continue;
+        continue; // LCOV_EXCL_LINE
       }
       return r ? -1 : 0;
     }
diff --git a/src/common/compat_threads.c b/src/common/compat_threads.c
index 9c6fa55b32..f4809060d6 100644
--- a/src/common/compat_threads.c
+++ b/src/common/compat_threads.c
@@ -61,8 +61,8 @@ tor_cond_t *
 tor_cond_new(void)
 {
   tor_cond_t *cond = tor_malloc(sizeof(tor_cond_t));
-  if (tor_cond_init(cond)<0)
-    tor_free(cond);
+  if (BUG(tor_cond_init(cond)<0))
+    tor_free(cond); // LCOV_EXCL_LINE
   return cond;
 }
 
@@ -240,8 +240,11 @@ alert_sockets_create(alert_sockets_t *socks_out, uint32_t flags)
     if (socks[0] >= 0) {
       if (fcntl(socks[0], F_SETFD, FD_CLOEXEC) < 0 ||
           set_socket_nonblocking(socks[0]) < 0) {
+        // LCOV_EXCL_START -- if eventfd succeeds, fcntl will.
+        tor_assert_nonfatal_unreached();
         close(socks[0]);
         return -1;
+        // LCOV_EXCL_STOP
       }
     }
   }
@@ -275,9 +278,12 @@ alert_sockets_create(alert_sockets_t *socks_out, uint32_t flags)
         fcntl(socks[1], F_SETFD, FD_CLOEXEC) < 0 ||
         set_socket_nonblocking(socks[0]) < 0 ||
         set_socket_nonblocking(socks[1]) < 0) {
+      // LCOV_EXCL_START -- if pipe succeeds, you can fcntl the output
+      tor_assert_nonfatal_unreached();
       close(socks[0]);
       close(socks[1]);
       return -1;
+      // LCOV_EXCL_STOP
     }
     socks_out->read_fd = socks[0];
     socks_out->write_fd = socks[1];
@@ -292,9 +298,12 @@ alert_sockets_create(alert_sockets_t *socks_out, uint32_t flags)
       tor_socketpair(AF_UNIX, SOCK_STREAM, 0, socks) == 0) {
     if (set_socket_nonblocking(socks[0]) < 0 ||
         set_socket_nonblocking(socks[1])) {
+      // LCOV_EXCL_START -- if socketpair worked, you can make it nonblocking.
+      tor_assert_nonfatal_unreached();
       tor_close_socket(socks[0]);
       tor_close_socket(socks[1]);
       return -1;
+      // LCOV_EXCL_STOP
     }
     socks_out->read_fd = socks[0];
     socks_out->write_fd = socks[1];
diff --git a/src/common/workqueue.c b/src/common/workqueue.c
index 0a38550de0..48c0cca01f 100644
--- a/src/common/workqueue.c
+++ b/src/common/workqueue.c
@@ -262,9 +262,12 @@ workerthread_new(void *state, threadpool_t *pool, replyqueue_t *replyqueue)
   thr->in_pool = pool;
 
   if (spawn_func(worker_thread_main, thr) < 0) {
+    //LCOV_EXCL_START
+    tor_assert_nonfatal_unreached();
     log_err(LD_GENERAL, "Can't launch worker thread.");
     tor_free(thr);
     return NULL;
+    //LCOV_EXCL_STOP
   }
 
   return thr;
@@ -375,8 +378,8 @@ threadpool_queue_update(threadpool_t *pool,
 static int
 threadpool_start_threads(threadpool_t *pool, int n)
 {
-  if (n < 0)
-    return -1;
+  if (BUG(n < 0))
+    return -1; // LCOV_EXCL_LINE
   if (n > MAX_THREADS)
     n = MAX_THREADS;
 
@@ -391,9 +394,12 @@ threadpool_start_threads(threadpool_t *pool, int n)
     workerthread_t *thr = workerthread_new(state, pool, pool->reply_queue);
 
     if (!thr) {
+      //LCOV_EXCL_START
+      tor_assert_nonfatal_unreached();
       pool->free_thread_state_fn(state);
       tor_mutex_release(&pool->lock);
       return -1;
+      //LCOV_EXCL_STOP
     }
     thr->index = pool->n_threads;
     pool->threads[pool->n_threads++] = thr;
@@ -429,10 +435,13 @@ threadpool_new(int n_threads,
   pool->reply_queue = replyqueue;
 
   if (threadpool_start_threads(pool, n_threads) < 0) {
+    //LCOV_EXCL_START
+    tor_assert_nonfatal_unreached();
     tor_cond_uninit(&pool->condition);
     tor_mutex_uninit(&pool->lock);
     tor_free(pool);
     return NULL;
+    //LCOV_EXCL_STOP
   }
 
   return pool;
@@ -456,8 +465,10 @@ replyqueue_new(uint32_t alertsocks_flags)
 
   rq = tor_malloc_zero(sizeof(replyqueue_t));
   if (alert_sockets_create(&rq->alert, alertsocks_flags) < 0) {
+    //LCOV_EXCL_START
     tor_free(rq);
     return NULL;
+    //LCOV_EXCL_STOP
   }
 
   tor_mutex_init(&rq->lock);
@@ -486,10 +497,12 @@ void
 replyqueue_process(replyqueue_t *queue)
 {
   if (queue->alert.drain_fn(queue->alert.read_fd) < 0) {
+    //LCOV_EXCL_START
     static ratelim_t warn_limit = RATELIM_INIT(7200);
     log_fn_ratelim(&warn_limit, LOG_WARN, LD_GENERAL,
                  "Failure from drain_fd: %s",
                  tor_socket_strerror(tor_socket_errno(queue->alert.read_fd)));
+    //LCOV_EXCL_STOP
   }
 
   tor_mutex_acquire(&queue->lock);
diff --git a/src/test/include.am b/src/test/include.am
index fa660f5302..5a91c74cde 100644
--- a/src/test/include.am
+++ b/src/test/include.am
@@ -9,6 +9,12 @@ TESTS_ENVIRONMENT = \
 	export TESTING_TOR_BINARY="$(TESTING_TOR_BINARY)";
 
 TESTSCRIPTS = src/test/test_zero_length_keys.sh \
+	src/test/test_workqueue_cancel.sh \
+	src/test/test_workqueue_efd.sh \
+	src/test/test_workqueue_efd2.sh \
+	src/test/test_workqueue_pipe.sh \
+	src/test/test_workqueue_pipe2.sh \
+	src/test/test_workqueue_socketpair.sh \
 	src/test/test_switch_id.sh
 
 if USEPYTHON
@@ -16,7 +22,8 @@ TESTSCRIPTS += src/test/test_ntor.sh src/test/test_bt.sh
 endif
 
 TESTS += src/test/test src/test/test-slow src/test/test-memwipe \
-	src/test/test_workqueue src/test/test_keygen.sh \
+	src/test/test_workqueue \
+	src/test/test_keygen.sh \
 	src/test/test-timers \
 	$(TESTSCRIPTS)
 
@@ -257,4 +264,11 @@ EXTRA_DIST += \
 	src/test/test_zero_length_keys.sh \
 	src/test/test_ntor.sh src/test/test_bt.sh \
 	src/test/test-network.sh \
-	src/test/test_switch_id.sh
+	src/test/test_switch_id.sh \
+	src/test/test_workqueue_cancel.sh \
+	src/test/test_workqueue_efd.sh \
+	src/test/test_workqueue_efd2.sh \
+	src/test/test_workqueue_pipe.sh \
+	src/test/test_workqueue_pipe2.sh \
+	src/test/test_workqueue_socketpair.sh
+
diff --git a/src/test/test_workqueue.c b/src/test/test_workqueue.c
index cbcf596b22..75d68a7ce0 100644
--- a/src/test/test_workqueue.c
+++ b/src/test/test_workqueue.c
@@ -400,6 +400,9 @@ main(int argc, char **argv)
   }
 
   rq = replyqueue_new(as_flags);
+  if (as_flags && rq == NULL)
+    return 77; // 77 means "skipped".
+
   tor_assert(rq);
   tp = threadpool_new(opt_n_threads,
                       rq, new_state, free_state, NULL);
diff --git a/src/test/test_workqueue_cancel.sh b/src/test/test_workqueue_cancel.sh
new file mode 100755
index 0000000000..f7c663171e
--- /dev/null
+++ b/src/test/test_workqueue_cancel.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue -C 1
+
diff --git a/src/test/test_workqueue_efd.sh b/src/test/test_workqueue_efd.sh
new file mode 100755
index 0000000000..4d89396819
--- /dev/null
+++ b/src/test/test_workqueue_efd.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue \
+	   --no-eventfd2 --no-pipe2 --no-pipe --no-socketpair
diff --git a/src/test/test_workqueue_efd2.sh b/src/test/test_workqueue_efd2.sh
new file mode 100755
index 0000000000..7cfff45ff3
--- /dev/null
+++ b/src/test/test_workqueue_efd2.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue \
+	   --no-eventfd --no-pipe2 --no-pipe --no-socketpair
diff --git a/src/test/test_workqueue_pipe.sh b/src/test/test_workqueue_pipe.sh
new file mode 100755
index 0000000000..afcef87853
--- /dev/null
+++ b/src/test/test_workqueue_pipe.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue \
+	   --no-eventfd2 --no-eventfd --no-pipe2 --no-socketpair
diff --git a/src/test/test_workqueue_pipe2.sh b/src/test/test_workqueue_pipe2.sh
new file mode 100755
index 0000000000..a20a1427e0
--- /dev/null
+++ b/src/test/test_workqueue_pipe2.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue \
+	   --no-eventfd2 --no-eventfd --no-pipe --no-socketpair
diff --git a/src/test/test_workqueue_socketpair.sh b/src/test/test_workqueue_socketpair.sh
new file mode 100755
index 0000000000..76af79746d
--- /dev/null
+++ b/src/test/test_workqueue_socketpair.sh
@@ -0,0 +1,4 @@
+#!/bin/sh
+
+${builddir:-.}/src/test/test_workqueue \
+	   --no-eventfd2 --no-eventfd --no-pipe2 --no-pipe