Fix a rare infinite-recursion bug when shutting down.

Once we had called log_free_all(), anything that tried to log a
message (like a failed tor_assert()) would fail like this:

   1. The logging call eventually invokes the _log() function.
   2. _log() calls tor_mutex_lock(log_mutex).
   3. tor_mutex_lock(m) calls tor_assert(m).
   4. Since we freed the log_mutex, tor_assert() fails, and tries to
      log its failure.
   5. GOTO 1.

Now we allocate the mutex statically, and never destroy it on
shutdown.

Bugfix on 0.2.0.16-alpha, which introduced the log mutex.

This bug was found by Matt Edman.

diff --git a/ChangeLog b/ChangeLog
index d64423426e2322896bf021dd6bd1604b47868270..842f05c1c4fecf67c8a4b9526907917907a84c6e 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -15,6 +15,9 @@ Changes in version 0.2.1.20 - 2009-??-??
     - Fix a signed/unsigned compile warning in 0.2.1.19.
     - Fix possible segmentation fault on directory authorities. Bugfix on
       0.2.1.14-rc.
+    - Fix an extremely infinite recursion bug that could occur if we tried
+      to log a message after shutting down the log subsystem.  Found by Matt
+      Edman.  Bugfix on 0.2.0.16-alpha.
 
 
 Changes in version 0.2.1.19 - 2009-07-28

diff --git a/src/common/log.c b/src/common/log.c
index a7b0c12c4a43a841695c815b1b93fa9e706a2ed4..ea09fca1672373cf4e069d6fb369e62375002d6e 100644 (file)
--- a/src/common/log.c
+++ b/src/common/log.c
@@ -94,7 +94,8 @@ should_log_function_name(log_domain_mask_t domain, int severity)
 }
 
 /** A mutex to guard changes to logfiles and logging. */
-static tor_mutex_t *log_mutex = NULL;
+static tor_mutex_t log_mutex;
+static int log_mutex_initialized = 0;
 
 /** Linked list of logfile_t. */
 static logfile_t *logfiles = NULL;
@@ -105,9 +106,9 @@ static int syslog_count = 0;
 #endif
 
 #define LOCK_LOGS() STMT_BEGIN                                          \
-  tor_mutex_acquire(log_mutex);                                         \
+  tor_mutex_acquire(&log_mutex);                                        \
   STMT_END
-#define UNLOCK_LOGS() STMT_BEGIN tor_mutex_release(log_mutex); STMT_END
+#define UNLOCK_LOGS() STMT_BEGIN 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.  */
@@ -448,8 +449,9 @@ logs_free_all(void)
     log_free(victim);
   }
   tor_free(appname);
-  tor_mutex_free(log_mutex);
-  log_mutex = NULL;
+
+  /* We _could_ destroy the log mutex here, but that would screw up any logs
+   * that happened between here and the end of execution. */
 }
 
 /** Remove and free the log entry <b>victim</b> from the linked-list
@@ -545,8 +547,10 @@ add_stream_log(const log_severity_list_t *severity,
 void
 init_logging(void)
 {
-  if (!log_mutex)
-    log_mutex = tor_mutex_new();
+  if (!log_mutex_initialized) {
+    tor_mutex_init(&log_mutex);
+    log_mutex_initialized = 1;
+  }
 }
 
 /** Add a log handler to receive messages during startup (before the real