Browse Source

Changed threads to be joinable rather than detatched

Threads are now joinable, which means 'spawn_func' now returns a
new 'tor_thread_t' object. This thread object can be joined using
'join_thread(t)'. The Windows implementation has not been tested.

This will make it easier to safely stop a thread and make sure
that thread is not accessing tor's global memory as tor is
shutting down.
Steven Engler 4 years ago
parent
commit
135e50cdd1

+ 1 - 1
src/app/main/tor_threads.c

@@ -30,7 +30,7 @@ tor_thread_helper(void *_info)
   spawn_exit();
 }
 
-int
+tor_thread_t *
 start_tor_thread(void (*func)(void *), void *data)
 {
   tor_thread_helper_info_t *d = tor_malloc(sizeof(tor_thread_helper_info_t));

+ 3 - 1
src/app/main/tor_threads.h

@@ -6,6 +6,8 @@
 #ifndef TOR_APP_THREADS_H
 #define TOR_APP_THREADS_H
 
-int start_tor_thread(void (*func)(void *), void *data);
+#include "lib/thread/threads.h"
+
+tor_thread_t *start_tor_thread(void (*func)(void *), void *data);
 
 #endif /* !defined(TOR_APP_THREADS_H) */

+ 24 - 16
src/lib/thread/compat_pthreads.c

@@ -51,10 +51,6 @@ tor_pthread_helper_fn(void *_data)
   func(arg);
   return NULL;
 }
-/**
- * A pthread attribute to make threads start detached.
- */
-static pthread_attr_t attr_detached;
 /** True iff we've called tor_threads_init() */
 static int threads_initialized = 0;
 
@@ -68,7 +64,7 @@ static int threads_initialized = 0;
  * the caller's stack will still be around when the called function is
  * running.
  */
-int
+tor_thread_t*
 spawn_func(void (*func)(void *), void *data)
 {
   pthread_t thread;
@@ -79,12 +75,14 @@ spawn_func(void (*func)(void *), void *data)
   d = tor_malloc(sizeof(tor_pthread_data_t));
   d->data = data;
   d->func = func;
-  if (pthread_create(&thread, &attr_detached, tor_pthread_helper_fn, d)) {
+  if (pthread_create(&thread, NULL, tor_pthread_helper_fn, d)) {
     tor_free(d);
-    return -1;
+    return NULL;
   }
 
-  return 0;
+  tor_thread_t* tor_thread = tor_malloc(sizeof(tor_thread_t));
+  tor_thread->pthread = thread;
+  return tor_thread;
 }
 
 /** End the current thread/process.
@@ -95,6 +93,24 @@ spawn_exit(void)
   pthread_exit(NULL);
 }
 
+/** Join the thread and return -1 on error, or 0 if okay. */
+int
+join_thread(tor_thread_t* thread)
+{
+  int rv = pthread_join(thread->pthread, NULL);
+  if (rv != 0) {
+    return -1;
+  }
+  return 0;
+}
+
+/** Free the thread. */
+void
+free_thread(tor_thread_t* thread)
+{
+  tor_free(thread);
+}
+
 /** Return an integer representing this thread. */
 unsigned long
 tor_get_thread_id(void)
@@ -256,14 +272,6 @@ tor_threads_init(void)
 {
   if (!threads_initialized) {
     tor_locking_init();
-    const int ret1 = pthread_attr_init(&attr_detached);
-    tor_assert(ret1 == 0);
-#ifndef PTHREAD_CREATE_DETACHED
-#define PTHREAD_CREATE_DETACHED 1
-#endif
-    const int ret2 =
-      pthread_attr_setdetachstate(&attr_detached, PTHREAD_CREATE_DETACHED);
-    tor_assert(ret2 == 0);
     threads_initialized = 1;
     set_main_thread();
   }

+ 39 - 6
src/lib/thread/compat_winthreads.c

@@ -31,14 +31,18 @@
  * the caller's stack will still be around when the called function is
  * running.
  */
-int
+tor_thread*
 spawn_func(void (*func)(void *), void *data)
 {
-  int rv;
-  rv = (int)_beginthread(func, 0, data);
-  if (rv == (int)-1)
-    return -1;
-  return 0;
+  uintptr_t thread;
+  thread = _beginthread(func, 0, data);
+  if ((int)thread == -1) {
+    return NULL;
+  }
+
+  tor_thread_t* tor_thread = tor_malloc(sizeof(tor_thread_t));
+  tor_thread->winthread = thread;
+  return tor_thread;
 }
 
 /** End the current thread/process.
@@ -55,6 +59,35 @@ spawn_exit(void)
   // LCOV_EXCL_STOP
 }
 
+/** Join the thread and return -1 on error, or 0 if okay. */
+int
+join_thread(tor_thread* thread)
+{
+  DWORD rv = WaitForSingleObject(thread->winthread, INFINITE);
+  if (rv != WAIT_OBJECT_0) {
+    DWORD err = GetLastError();
+    char *msg = format_win32_error(err);
+    log_err(LD_GENERAL, "Error joining thread: %s", msg);
+    tor_free(msg);
+    return -1;
+  }
+  return 0;
+}
+
+/** Free the thread. */
+void
+free_thread(tor_thread* thread)
+{
+  if (CloseHandle(thread->winthread) != 0) {
+    DWORD err = GetLastError();
+    char *msg = format_win32_error(err);
+    log_err(LD_GENERAL, "Error freeing thread: %s", msg);
+    tor_free(msg);
+    tor_assert(0);
+  }
+  tor_free(thread);
+}
+
 unsigned long
 tor_get_thread_id(void)
 {

+ 11 - 1
src/lib/thread/threads.h

@@ -26,8 +26,18 @@
 
 struct timeval;
 
-int spawn_func(void (*func)(void *), void *data);
+typedef struct tor_thread_s {
+#ifdef _WIN32
+  uintptr_t winthread;
+#else
+  pthread_t pthread;
+#endif
+} tor_thread_t;
+
+tor_thread_t* spawn_func(void (*func)(void *), void *data);
 void spawn_exit(void) ATTR_NORETURN;
+int join_thread(tor_thread_t* thread);
+void free_thread(tor_thread_t* thread);
 
 unsigned long tor_get_thread_id(void);
 void tor_threads_init(void);