Browse Source

Merge branch 'check_log_mutex_uncherrypicked'

Nick Mathewson 8 years ago
parent
commit
fed8c5199a
4 changed files with 19 additions and 4 deletions
  1. 5 0
      changes/assert_event_base
  2. 1 0
      src/common/compat_libevent.c
  3. 10 3
      src/common/log.c
  4. 3 1
      src/test/testing_common.c

+ 5 - 0
changes/assert_event_base

@@ -0,0 +1,5 @@
+  o Minor features (robustness):
+    - Exit immediately with an error message if the code attempts to
+      use libevent without having initialized it. This should resolve
+      some frequently-made mistakes in our unit tests. Closes ticket
+      18241.

+ 1 - 0
src/common/compat_libevent.c

@@ -247,6 +247,7 @@ tor_libevent_initialize(tor_libevent_cfg *torcfg)
 MOCK_IMPL(struct event_base *,
 tor_libevent_get_base, (void))
 {
+  tor_assert(the_event_base != NULL);
   return the_event_base;
 }
 

+ 10 - 3
src/common/log.c

@@ -149,10 +149,14 @@ static int pretty_fn_has_parens = 0;
 
 /** Lock the log_mutex to prevent others from changing the logfile_t list */
 #define LOCK_LOGS() STMT_BEGIN                                          \
+  tor_assert(log_mutex_initialized);                                    \
   tor_mutex_acquire(&log_mutex);                                        \
   STMT_END
 /** Unlock the log_mutex */
-#define UNLOCK_LOGS() STMT_BEGIN tor_mutex_release(&log_mutex); STMT_END
+#define UNLOCK_LOGS() STMT_BEGIN                                        \
+  tor_assert(log_mutex_initialized);                                    \
+  tor_mutex_release(&log_mutex);                                        \
+  STMT_END
 
 /** What's the lowest log level anybody cares about?  Checking this lets us
  * bail out early from log_debug if we aren't debugging.  */
@@ -482,9 +486,12 @@ logv,(int severity, log_domain_mask_t domain, const char *funcname,
   /* check that severity is sane.  Overrunning the masks array leads to
    * interesting and hard to diagnose effects */
   assert(severity >= LOG_ERR && severity <= LOG_DEBUG);
+  /* check that we've initialised the log mutex before we try to lock it */
+  assert(log_mutex_initialized);
   LOCK_LOGS();
 
-  if ((! (domain & LD_NOCB)) && smartlist_len(pending_cb_messages))
+  if ((! (domain & LD_NOCB)) && pending_cb_messages
+      && smartlist_len(pending_cb_messages))
     flush_pending_log_callbacks();
 
   if (queue_startup_messages &&
@@ -939,7 +946,7 @@ flush_pending_log_callbacks(void)
   smartlist_t *messages, *messages_tmp;
 
   LOCK_LOGS();
-  if (0 == smartlist_len(pending_cb_messages)) {
+  if (!pending_cb_messages || 0 == smartlist_len(pending_cb_messages)) {
     UNLOCK_LOGS();
     return;
   }

+ 3 - 1
src/test/testing_common.c

@@ -228,6 +228,9 @@ main(int c, const char **v)
   int loglevel = LOG_ERR;
   int accel_crypto = 0;
 
+  /* We must initialise logs before we call tor_assert() */
+  init_logging(1);
+
 #ifdef USE_DMALLOC
   {
     int r = CRYPTO_set_mem_ex_functions(tor_malloc_, tor_realloc_, tor_free_);
@@ -244,7 +247,6 @@ main(int c, const char **v)
   tor_libevent_initialize(&cfg);
 
   control_initialize_event_queue();
-  init_logging(1);
   configure_backtrace_handler(get_version());
 
   for (i_out = i = 1; i < c; ++i) {