Extract the locking and logging code

The locking code gets its own module, since it's more fundamental
than the higher-level locking code.

Extracting the logging code was the whole point here. :)

.gitignore

@@ -175,6 +175,8 @@ uptime-*.json
 /src/lib/libtor-err-testing.a
 /src/lib/libtor-intmath.a
 /src/lib/libtor-intmath-testing.a
+/src/lib/libtor-lock.a
+/src/lib/libtor-lock-testing.a
 /src/lib/libtor-malloc.a
 /src/lib/libtor-malloc-testing.a
 /src/lib/libtor-string.a

Makefile.am

@@ -40,6 +40,8 @@ endif
 # "Common" libraries used to link tor's utility code.
 TOR_UTIL_LIBS = \
 	src/common/libor.a \
+        src/lib/libtor-log.a \
+        src/lib/libtor-lock.a \
 	src/lib/libtor-container.a \
         src/lib/libtor-string.a \
 	src/lib/libtor-malloc.a \
@@ -52,6 +54,8 @@ TOR_UTIL_LIBS = \
 # and tests)
 TOR_UTIL_TESTING_LIBS = \
 	src/common/libor-testing.a \
+        src/lib/libtor-log-testing.a \
+        src/lib/libtor-lock-testing.a \
 	src/lib/libtor-container-testing.a \
         src/lib/libtor-string-testing.a \
 	src/lib/libtor-malloc-testing.a \

src/common/compat_pthreads.c

@@ -91,83 +91,6 @@ spawn_exit(void)
   pthread_exit(NULL);
 }
 
-/** A mutex attribute that we're going to use to tell pthreads that we want
- * "recursive" mutexes (i.e., once we can re-lock if we're already holding
- * them.) */
-static pthread_mutexattr_t attr_recursive;
-
-/** Initialize <b>mutex</b> so it can be locked.  Every mutex must be set
- * up with tor_mutex_init() or tor_mutex_new(); not both. */
-void
-tor_mutex_init(tor_mutex_t *mutex)
-{
-  if (PREDICT_UNLIKELY(!threads_initialized))
-    tor_threads_init(); // LCOV_EXCL_LINE
-  const int err = pthread_mutex_init(&mutex->mutex, &attr_recursive);
-  if (PREDICT_UNLIKELY(err)) {
-    // LCOV_EXCL_START
-    raw_assert_unreached_msg("Error creating a mutex.");
-    // LCOV_EXCL_STOP
-  }
-}
-
-/** As tor_mutex_init, but initialize a mutex suitable that may be
- * non-recursive, if the OS supports that. */
-void
-tor_mutex_init_nonrecursive(tor_mutex_t *mutex)
-{
-  int err;
-  if (!threads_initialized)
-    tor_threads_init(); // LCOV_EXCL_LINE
-  err = pthread_mutex_init(&mutex->mutex, NULL);
-  if (PREDICT_UNLIKELY(err)) {
-    // LCOV_EXCL_START
-    raw_assert_unreached_msg("Error creating a mutex.");
-    // LCOV_EXCL_STOP
-  }
-}
-
-/** Wait until <b>m</b> is free, then acquire it. */
-void
-tor_mutex_acquire(tor_mutex_t *m)
-{
-  int err;
-  raw_assert(m);
-  err = pthread_mutex_lock(&m->mutex);
-  if (PREDICT_UNLIKELY(err)) {
-    // LCOV_EXCL_START
-    raw_assert_unreached_msg("Error locking a mutex.");
-    // LCOV_EXCL_STOP
-  }
-}
-/** Release the lock <b>m</b> so another thread can have it. */
-void
-tor_mutex_release(tor_mutex_t *m)
-{
-  int err;
-  raw_assert(m);
-  err = pthread_mutex_unlock(&m->mutex);
-  if (PREDICT_UNLIKELY(err)) {
-    // LCOV_EXCL_START
-    raw_assert_unreached_msg("Error unlocking a mutex.");
-    // LCOV_EXCL_STOP
-  }
-}
-/** Clean up the mutex <b>m</b> so that it no longer uses any system
- * resources.  Does not free <b>m</b>.  This function must only be called on
- * mutexes from tor_mutex_init(). */
-void
-tor_mutex_uninit(tor_mutex_t *m)
-{
-  int err;
-  raw_assert(m);
-  err = pthread_mutex_destroy(&m->mutex);
-  if (PREDICT_UNLIKELY(err)) {
-    // LCOV_EXCL_START
-    raw_assert_unreached_msg("Error destroying a mutex.");
-    // LCOV_EXCL_STOP
-  }
-}
 /** Return an integer representing this thread. */
 unsigned long
 tor_get_thread_id(void)
@@ -328,8 +251,7 @@ void
 tor_threads_init(void)
 {
   if (!threads_initialized) {
-    pthread_mutexattr_init(&attr_recursive);
-    pthread_mutexattr_settype(&attr_recursive, PTHREAD_MUTEX_RECURSIVE);
+    tor_locking_init();
     const int ret1 = pthread_attr_init(&attr_detached);
     tor_assert(ret1 == 0);
 #ifndef PTHREAD_CREATE_DETACHED

src/common/compat_threads.c

@@ -29,33 +29,6 @@
 #include <unistd.h>
 #endif
 
-/** Return a newly allocated, ready-for-use mutex. */
-tor_mutex_t *
-tor_mutex_new(void)
-{
-  tor_mutex_t *m = tor_malloc_zero(sizeof(tor_mutex_t));
-  tor_mutex_init(m);
-  return m;
-}
-/** Return a newly allocated, ready-for-use mutex.  This one might be
- * non-recursive, if that's faster. */
-tor_mutex_t *
-tor_mutex_new_nonrecursive(void)
-{
-  tor_mutex_t *m = tor_malloc_zero(sizeof(tor_mutex_t));
-  tor_mutex_init_nonrecursive(m);
-  return m;
-}
-/** Release all storage and system resources held by <b>m</b>. */
-void
-tor_mutex_free_(tor_mutex_t *m)
-{
-  if (!m)
-    return;
-  tor_mutex_uninit(m);
-  tor_free(m);
-}
-
 /** Allocate and return a new condition variable. */
 tor_cond_t *
 tor_cond_new(void)
@@ -404,4 +377,3 @@ atomic_counter_exchange(atomic_counter_t *counter, size_t newval)
   return oldval;
 }
 #endif /* !defined(HAVE_STDATOMIC_H) */
-

src/common/compat_threads.h

@@ -9,54 +9,15 @@
 #include "orconfig.h"
 #include "lib/cc/torint.h"
 #include "lib/testsupport/testsupport.h"
-
-#if defined(HAVE_PTHREAD_H) && !defined(_WIN32)
-#include <pthread.h>
-#endif
+#include "lib/lock/compat_mutex.h"
 
 #ifdef HAVE_STDATOMIC_H
 #include <stdatomic.h>
 #endif
 
-#if defined(_WIN32)
-#define USE_WIN32_THREADS
-#elif defined(HAVE_PTHREAD_H) && defined(HAVE_PTHREAD_CREATE)
-#define USE_PTHREADS
-#else
-#error "No threading system was found"
-#endif /* defined(_WIN32) || ... */
-
 int spawn_func(void (*func)(void *), void *data);
 void spawn_exit(void) ATTR_NORETURN;
 
-/* Because we use threads instead of processes on most platforms (Windows,
- * Linux, etc), we need locking for them.  On platforms with poor thread
- * support or broken gethostbyname_r, these functions are no-ops. */
-
-/** A generic lock structure for multithreaded builds. */
-typedef struct tor_mutex_t {
-#if defined(USE_WIN32_THREADS)
-  /** Windows-only: on windows, we implement locks with CRITICAL_SECTIONS. */
-  CRITICAL_SECTION mutex;
-#elif defined(USE_PTHREADS)
-  /** Pthreads-only: with pthreads, we implement locks with
-   * pthread_mutex_t. */
-  pthread_mutex_t mutex;
-#else
-  /** No-threads only: Dummy variable so that tor_mutex_t takes up space. */
-  int _unused;
-#endif /* defined(USE_WIN32_THREADS) || ... */
-} tor_mutex_t;
-
-tor_mutex_t *tor_mutex_new(void);
-tor_mutex_t *tor_mutex_new_nonrecursive(void);
-void tor_mutex_init(tor_mutex_t *m);
-void tor_mutex_init_nonrecursive(tor_mutex_t *m);
-void tor_mutex_acquire(tor_mutex_t *m);
-void tor_mutex_release(tor_mutex_t *m);
-void tor_mutex_free_(tor_mutex_t *m);
-#define tor_mutex_free(m) FREE_AND_NULL(tor_mutex_t, tor_mutex_free_, (m))
-void tor_mutex_uninit(tor_mutex_t *m);
 unsigned long tor_get_thread_id(void);
 void tor_threads_init(void);
 
@@ -220,4 +181,3 @@ atomic_counter_exchange(atomic_counter_t *counter, size_t newval)
 #endif /* defined(HAVE_STDATOMIC_H) */
 
 #endif /* !defined(TOR_COMPAT_THREADS_H) */
-

src/common/compat_winthreads.c

@@ -54,33 +54,6 @@ spawn_exit(void)
   // LCOV_EXCL_STOP
 }
 
-void
-tor_mutex_init(tor_mutex_t *m)
-{
-  InitializeCriticalSection(&m->mutex);
-}
-void
-tor_mutex_init_nonrecursive(tor_mutex_t *m)
-{
-  InitializeCriticalSection(&m->mutex);
-}
-
-void
-tor_mutex_uninit(tor_mutex_t *m)
-{
-  DeleteCriticalSection(&m->mutex);
-}
-void
-tor_mutex_acquire(tor_mutex_t *m)
-{
-  raw_assert(m);
-  EnterCriticalSection(&m->mutex);
-}
-void
-tor_mutex_release(tor_mutex_t *m)
-{
-  LeaveCriticalSection(&m->mutex);
-}
 unsigned long
 tor_get_thread_id(void)
 {

src/common/include.am

@@ -38,7 +38,6 @@ LIBOR_A_SRC = \
   src/common/compat_threads.c				\
   src/common/compat_time.c				\
   src/common/confline.c					\
-  src/common/log.c					\
   src/common/memarea.c					\
   src/common/util.c					\
   src/common/util_bug.c					\
@@ -94,7 +93,6 @@ COMMONHEADERS = \
   src/common/storagedir.h			\
   src/common/timers.h				\
   src/common/token_bucket.h			\
-  src/common/torlog.h				\
   src/common/util.h				\
   src/common/util_bug.h				\
   src/common/util_format.h			\

src/common/util.c

@@ -1309,57 +1309,6 @@ format_time_interval(char *out, size_t out_len, long interval)
   }
 }
 
-/* =====
- * Rate limiting
- * ===== */
-
-/** If the rate-limiter <b>lim</b> is ready at <b>now</b>, return the number
- * of calls to rate_limit_is_ready (including this one!) since the last time
- * rate_limit_is_ready returned nonzero.  Otherwise return 0.
- * If the call number hits <b>RATELIM_TOOMANY</b> limit, drop a warning
- * about this event and stop counting. */
-static int
-rate_limit_is_ready(ratelim_t *lim, time_t now)
-{
-  if (lim->rate + lim->last_allowed <= now) {
-    int res = lim->n_calls_since_last_time + 1;
-    lim->last_allowed = now;
-    lim->n_calls_since_last_time = 0;
-    return res;
-  } else {
-    if (lim->n_calls_since_last_time <= RATELIM_TOOMANY) {
-      ++lim->n_calls_since_last_time;
-    }
-
-    return 0;
-  }
-}
-
-/** If the rate-limiter <b>lim</b> is ready at <b>now</b>, return a newly
- * allocated string indicating how many messages were suppressed, suitable to
- * append to a log message.  Otherwise return NULL. */
-char *
-rate_limit_log(ratelim_t *lim, time_t now)
-{
-  int n;
-  if ((n = rate_limit_is_ready(lim, now))) {
-    if (n == 1) {
-      return tor_strdup("");
-    } else {
-      char *cp=NULL;
-      const char *opt_over = (n >= RATELIM_TOOMANY) ? "over " : "";
-      /* XXXX this is not exactly correct: the messages could have occurred
-       * any time between the old value of lim->allowed and now. */
-      tor_asprintf(&cp,
-                   " [%s%d similar message(s) suppressed in last %d seconds]",
-                   opt_over, n-1, lim->rate);
-      return cp;
-    }
-  } else {
-    return NULL;
-  }
-}
-
 /* =====
  * File helpers
  * ===== */

src/common/util.h

@@ -126,43 +126,6 @@ int parse_iso_time_nospace(const char *cp, time_t *t);
 int parse_http_time(const char *buf, struct tm *tm);
 int format_time_interval(char *out, size_t out_len, long interval);
 
-/* Rate-limiter */
-
-/** A ratelim_t remembers how often an event is occurring, and how often
- * it's allowed to occur.  Typical usage is something like:
- *
-   <pre>
-    if (possibly_very_frequent_event()) {
-      const int INTERVAL = 300;
-      static ratelim_t warning_limit = RATELIM_INIT(INTERVAL);
-      char *m;
-      if ((m = rate_limit_log(&warning_limit, approx_time()))) {
-        log_warn(LD_GENERAL, "The event occurred!%s", m);
-        tor_free(m);
-      }
-    }
-   </pre>
-
-   As a convenience wrapper for logging, you can replace the above with:
-   <pre>
-   if (possibly_very_frequent_event()) {
-     static ratelim_t warning_limit = RATELIM_INIT(300);
-     log_fn_ratelim(&warning_limit, LOG_WARN, LD_GENERAL,
-                    "The event occurred!");
-   }
-   </pre>
- */
-typedef struct ratelim_t {
-  int rate;
-  time_t last_allowed;
-  int n_calls_since_last_time;
-} ratelim_t;
-
-#define RATELIM_INIT(r) { (r), 0, 0 }
-#define RATELIM_TOOMANY (16*1000*1000)
-
-char *rate_limit_log(ratelim_t *lim, time_t now);
-
 /* File helpers */
 ssize_t write_all(tor_socket_t fd, const char *buf, size_t count,int isSocket);
 ssize_t read_all(tor_socket_t fd, char *buf, size_t count, int isSocket);

src/include.am

@@ -8,6 +8,8 @@ include src/lib/crypt_ops/include.am
 include src/lib/defs/include.am
 include src/lib/include.libdonna.am
 include src/lib/intmath/include.am
+include src/lib/lock/include.am
+include src/lib/log/include.am
 include src/lib/malloc/include.am
 include src/lib/string/include.am
 include src/lib/testsupport/include.am

src/lib/lock/.may_include

@@ -0,0 +1,5 @@
+orconfig.h
+lib/cc/*.h
+lib/err/*.h
+lib/lock/*.h
+lib/malloc/*.h