[or-cvs] [tor/master] Don't attempt to log messages to a controller from a worker thread.

[Nick Mathewson]

Author: Nick Mathewson <nickm at torproject.org>
Date: Fri, 29 May 2009 10:18:50 -0400
Subject: Don't attempt to log messages to a controller from a worker thread.
Commit: fd992deeea769721dc95bacb40ea360ef42f76dd

This patch adds a function to determine whether we're in the main
thread, and changes control_event_logmsg() to return immediately if
we're in a subthread.  This is necessary because otherwise we will
call connection_write_to_buf, which modifies non-locked data
structures.

Bugfix on 0.2.0.x; fix for at least one of the things currently
called "bug 977".
---
 ChangeLog           |    3 +++
 src/common/compat.c |   19 +++++++++++++++++++
 src/common/compat.h |    3 +++
 src/common/util.c   |    2 ++
 src/or/control.c    |    5 +++++
 5 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 709f747..1846832 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,9 @@ Changes in version 0.2.1.16-?? - 2009-??-??
     - Don't warn users about low port and hibernation mix when they
       provide a *ListenAddress directive to fix that. Bugfix on
       0.2.1.15-rc.
+    - Fix a race condition that could cause crashes or memory
+      corruption when running as a server with a controller listening
+      for log messages.
 
   o Minor bugfixes (on 0.2.1.x):
     - When switching back and forth between bridge mode, do not start
diff --git a/src/common/compat.c b/src/common/compat.c
index 35bb3a9..d62b1ce 100644
--- a/src/common/compat.c
+++ b/src/common/compat.c
@@ -2076,6 +2076,7 @@ tor_threads_init(void)
     pthread_mutexattr_init(&attr_reentrant);
     pthread_mutexattr_settype(&attr_reentrant, PTHREAD_MUTEX_RECURSIVE);
     threads_initialized = 1;
+    set_main_thread();
   }
 }
 #elif defined(USE_WIN32_THREADS)
@@ -2168,9 +2169,27 @@ tor_threads_init(void)
 #if 0
   cond_event_tls_index = TlsAlloc();
 #endif
+  set_main_thread();
 }
 #endif
 
+/** Identity of the "main" thread */
+static unsigned long main_thread_id = -1;
+
+/** Start considering the current thread to be the 'main thread'.  This has
+ * no effect on anything besides in_main_thread(). */
+void
+set_main_thread(void)
+{
+  main_thread_id = tor_get_thread_id();
+}
+/** Return true iff called from the main thread. */
+int
+in_main_thread(void)
+{
+  return main_thread_id == tor_get_thread_id();
+}
+
 /**
  * On Windows, WSAEWOULDBLOCK is not always correct: when you see it,
  * you need to ask the socket for its actual errno.  Also, you need to
diff --git a/src/common/compat.h b/src/common/compat.h
index edc38fa..4d5a016 100644
--- a/src/common/compat.h
+++ b/src/common/compat.h
@@ -522,6 +522,9 @@ void tor_threads_init(void);
 #define tor_threads_init() STMT_NIL
 #endif
 
+void set_main_thread(void);
+int in_main_thread(void);
+
 #ifdef TOR_IS_MULTITHREADED
 #if 0
 typedef struct tor_cond_t tor_cond_t;
diff --git a/src/common/util.c b/src/common/util.c
index a3338b1..7b9e5eb 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -2480,6 +2480,8 @@ start_daemon(void)
     if (fork() != 0) {
       exit(0);
     }
+    set_main_thread(); /* We are now the main thread. */
+
     return;
   }
 }
diff --git a/src/or/control.c b/src/or/control.c
index 8ded315..486ccc4 100644
--- a/src/or/control.c
+++ b/src/or/control.c
@@ -3367,6 +3367,11 @@ control_event_logmsg(int severity, uint32_t domain, const char *msg)
 {
   int event;
 
+  /* Don't even think of trying to add stuff to a buffer from a cpuworker
+   * thread. */
+  if (! in_main_thread())
+    return;
+
   if (disable_log_messages)
     return;
 
-- 
1.5.6.5