diff options
Diffstat (limited to 'src/lib/thread')
-rw-r--r-- | src/lib/thread/.may_include | 1 | ||||
-rw-r--r-- | src/lib/thread/compat_pthreads.c | 2 | ||||
-rw-r--r-- | src/lib/thread/compat_threads.c | 29 | ||||
-rw-r--r-- | src/lib/thread/compat_winthreads.c | 126 | ||||
-rw-r--r-- | src/lib/thread/include.am | 7 | ||||
-rw-r--r-- | src/lib/thread/lib_thread.md | 7 | ||||
-rw-r--r-- | src/lib/thread/numcpus.c | 2 | ||||
-rw-r--r-- | src/lib/thread/numcpus.h | 4 | ||||
-rw-r--r-- | src/lib/thread/thread_sys.h | 14 | ||||
-rw-r--r-- | src/lib/thread/threading.md | 26 | ||||
-rw-r--r-- | src/lib/thread/threads.h | 29 |
11 files changed, 140 insertions, 107 deletions
diff --git a/src/lib/thread/.may_include b/src/lib/thread/.may_include index fc56f46836..02711348c5 100644 --- a/src/lib/thread/.may_include +++ b/src/lib/thread/.may_include @@ -2,6 +2,7 @@ orconfig.h lib/cc/*.h lib/lock/*.h lib/log/*.h +lib/subsys/*.h lib/testsupport/*.h lib/thread/*.h lib/wallclock/*.h diff --git a/src/lib/thread/compat_pthreads.c b/src/lib/thread/compat_pthreads.c index 6f7ecd17da..d143b80252 100644 --- a/src/lib/thread/compat_pthreads.c +++ b/src/lib/thread/compat_pthreads.c @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** diff --git a/src/lib/thread/compat_threads.c b/src/lib/thread/compat_threads.c index 94ab021c52..75ade9c9f2 100644 --- a/src/lib/thread/compat_threads.c +++ b/src/lib/thread/compat_threads.c @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -14,9 +14,11 @@ #include "orconfig.h" #include <stdlib.h> #include "lib/thread/threads.h" +#include "lib/thread/thread_sys.h" #include "lib/log/log.h" #include "lib/log/util_bug.h" +#include "lib/subsys/subsys.h" #include <string.h> @@ -65,7 +67,15 @@ atomic_counter_init(atomic_counter_t *counter) memset(counter, 0, sizeof(*counter)); tor_mutex_init_nonrecursive(&counter->mutex); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ void atomic_counter_destroy(atomic_counter_t *counter) { @@ -109,3 +119,18 @@ atomic_counter_exchange(atomic_counter_t *counter, size_t newval) return oldval; } #endif /* !defined(HAVE_WORKING_STDATOMIC) */ + +static int +subsys_threads_initialize(void) +{ + tor_threads_init(); + return 0; +} + +const subsys_fns_t sys_threads = { + .name = "threads", + SUBSYS_DECLARE_LOCATION(), + .supported = true, + .level = -89, + .initialize = subsys_threads_initialize, +}; diff --git a/src/lib/thread/compat_winthreads.c b/src/lib/thread/compat_winthreads.c index f0b1430e84..fcc9c0279b 100644 --- a/src/lib/thread/compat_winthreads.c +++ b/src/lib/thread/compat_winthreads.c @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -10,18 +10,32 @@ * functions. */ +#include "orconfig.h" + #ifdef _WIN32 +/* For condition variable support */ +#ifndef WINVER +#error "orconfig.h didn't define WINVER" +#endif +#ifndef _WIN32_WINNT +#error "orconfig.h didn't define _WIN32_WINNT" +#endif +#if WINVER < 0x0600 +#error "winver too low" +#endif +#if _WIN32_WINNT < 0x0600 +#error "winver too low" +#endif #include <windows.h> #include <process.h> +#include <time.h> + #include "lib/thread/threads.h" #include "lib/log/log.h" #include "lib/log/util_bug.h" #include "lib/log/win32err.h" -/* This value is more or less total cargo-cult */ -#define SPIN_COUNT 2000 - /** Minimalist interface to run a void function in the background. On * Unix calls fork, on win32 calls beginthread. Returns -1 on failure. * func should not return, but rather should call spawn_exit. @@ -64,45 +78,24 @@ tor_get_thread_id(void) int tor_cond_init(tor_cond_t *cond) { - memset(cond, 0, sizeof(tor_cond_t)); - if (InitializeCriticalSectionAndSpinCount(&cond->lock, SPIN_COUNT)==0) { - return -1; - } - if ((cond->event = CreateEvent(NULL,TRUE,FALSE,NULL)) == NULL) { - DeleteCriticalSection(&cond->lock); - return -1; - } - cond->n_waiting = cond->n_to_wake = cond->generation = 0; + InitializeConditionVariable(&cond->cond); return 0; } void tor_cond_uninit(tor_cond_t *cond) { - DeleteCriticalSection(&cond->lock); - CloseHandle(cond->event); + (void) cond; } -static void -tor_cond_signal_impl(tor_cond_t *cond, int broadcast) -{ - EnterCriticalSection(&cond->lock); - if (broadcast) - cond->n_to_wake = cond->n_waiting; - else - ++cond->n_to_wake; - cond->generation++; - SetEvent(cond->event); - LeaveCriticalSection(&cond->lock); -} void tor_cond_signal_one(tor_cond_t *cond) { - tor_cond_signal_impl(cond, 0); + WakeConditionVariable(&cond->cond); } void tor_cond_signal_all(tor_cond_t *cond) { - tor_cond_signal_impl(cond, 1); + WakeAllConditionVariable(&cond->cond); } int @@ -152,66 +145,23 @@ int tor_cond_wait(tor_cond_t *cond, tor_mutex_t *lock_, const struct timeval *tv) { CRITICAL_SECTION *lock = &lock_->mutex; - int generation_at_start; - int waiting = 1; - int result = -1; - DWORD ms = INFINITE, ms_orig = INFINITE, startTime, endTime; - if (tv) - ms_orig = ms = tv->tv_sec*1000 + (tv->tv_usec+999)/1000; - - EnterCriticalSection(&cond->lock); - ++cond->n_waiting; - generation_at_start = cond->generation; - LeaveCriticalSection(&cond->lock); - - LeaveCriticalSection(lock); - - startTime = GetTickCount(); - do { - DWORD res; - res = WaitForSingleObject(cond->event, ms); - EnterCriticalSection(&cond->lock); - if (cond->n_to_wake && - cond->generation != generation_at_start) { - --cond->n_to_wake; - --cond->n_waiting; - result = 0; - waiting = 0; - goto out; - } else if (res != WAIT_OBJECT_0) { - result = (res==WAIT_TIMEOUT) ? 1 : -1; - --cond->n_waiting; - waiting = 0; - goto out; - } else if (ms != INFINITE) { - endTime = GetTickCount(); - if (startTime + ms_orig <= endTime) { - result = 1; /* Timeout */ - --cond->n_waiting; - waiting = 0; - goto out; - } else { - ms = startTime + ms_orig - endTime; - } - } - /* If we make it here, we are still waiting. */ - if (cond->n_to_wake == 0) { - /* There is nobody else who should wake up; reset - * the event. */ - ResetEvent(cond->event); - } - out: - LeaveCriticalSection(&cond->lock); - } while (waiting); - - EnterCriticalSection(lock); - - EnterCriticalSection(&cond->lock); - if (!cond->n_waiting) - ResetEvent(cond->event); - LeaveCriticalSection(&cond->lock); + DWORD ms = INFINITE; + if (tv) { + ms = tv->tv_sec*1000 + (tv->tv_usec+999)/1000; + } - return result; + BOOL ok = SleepConditionVariableCS(&cond->cond, lock, ms); + if (!ok) { + DWORD err = GetLastError(); + if (err == ERROR_TIMEOUT) { + return 1; + } + char *msg = format_win32_error(err); + log_err(LD_GENERAL, "Error waiting for condition variable: %s", msg); + tor_free(msg); + return -1; + } + return 0; } void diff --git a/src/lib/thread/include.am b/src/lib/thread/include.am index 9ec23d166e..cd8016b5df 100644 --- a/src/lib/thread/include.am +++ b/src/lib/thread/include.am @@ -12,6 +12,7 @@ if THREADS_WIN32 threads_impl_source=src/lib/thread/compat_winthreads.c endif +# ADD_C_FILE: INSERT SOURCES HERE. src_lib_libtor_thread_a_SOURCES = \ src/lib/thread/compat_threads.c \ src/lib/thread/numcpus.c \ @@ -22,6 +23,8 @@ src_lib_libtor_thread_testing_a_SOURCES = \ src_lib_libtor_thread_testing_a_CPPFLAGS = $(AM_CPPFLAGS) $(TEST_CPPFLAGS) src_lib_libtor_thread_testing_a_CFLAGS = $(AM_CFLAGS) $(TEST_CFLAGS) +# ADD_C_FILE: INSERT HEADERS HERE. noinst_HEADERS += \ - src/lib/thread/threads.h \ - src/lib/thread/numcpus.h + src/lib/thread/numcpus.h \ + src/lib/thread/thread_sys.h \ + src/lib/thread/threads.h diff --git a/src/lib/thread/lib_thread.md b/src/lib/thread/lib_thread.md new file mode 100644 index 0000000000..5870ad790f --- /dev/null +++ b/src/lib/thread/lib_thread.md @@ -0,0 +1,7 @@ +@dir /lib/thread +@brief lib/thread: Mid-level threading. + +This module contains compatibility and convenience code for multithreading, +except for low-level locks (which are in \refdir{lib/lock} and +workqueue/threadpool code (which belongs in \refdir{lib/evloop}.) + diff --git a/src/lib/thread/numcpus.c b/src/lib/thread/numcpus.c index b293d965d2..18454ce3ad 100644 --- a/src/lib/thread/numcpus.c +++ b/src/lib/thread/numcpus.c @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** diff --git a/src/lib/thread/numcpus.h b/src/lib/thread/numcpus.h index 3f0a29ce7c..65e6c430cf 100644 --- a/src/lib/thread/numcpus.h +++ b/src/lib/thread/numcpus.h @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -13,4 +13,4 @@ int compute_num_cpus(void); -#endif +#endif /* !defined(TOR_NUMCPUS_H) */ diff --git a/src/lib/thread/thread_sys.h b/src/lib/thread/thread_sys.h new file mode 100644 index 0000000000..6206fac9d6 --- /dev/null +++ b/src/lib/thread/thread_sys.h @@ -0,0 +1,14 @@ +/* Copyright (c) 2018-2020, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * \file thread_sys.h + * \brief Declare subsystem object for threads library + **/ + +#ifndef TOR_THREADS_SYS_H +#define TOR_THREADS_SYS_H + +extern const struct subsys_fns_t sys_threads; + +#endif /* !defined(TOR_THREADS_SYS_H) */ diff --git a/src/lib/thread/threading.md b/src/lib/thread/threading.md new file mode 100644 index 0000000000..a1058c97de --- /dev/null +++ b/src/lib/thread/threading.md @@ -0,0 +1,26 @@ + +@page threading Threading in Tor + +Tor is based around a single main thread and one or more worker +threads. We aim (with middling success) to use worker threads for +CPU-intensive activities and the main thread for our networking. +Fortunately (?) we have enough cryptography that moving what we can +of the cryptographic processes to the workers should achieve good +parallelism under most loads. Unfortunately, we only have a small +fraction of our cryptography done in our worker threads right now. + +Our threads-and-workers abstraction is defined in workqueue.c, which +combines a work queue with a thread pool, and integrates the +signalling with libevent. Tor's main instance of a work queue is +instantiated in cpuworker.c. It will probably need some refactoring +as more types of work are added. + +On a lower level, we provide locks with tor_mutex_t in \refdir{lib/lock}, and +higher-level locking/threading tools in \refdir{lib/thread}, including +conditions (tor_cond_t), thread-local storage (tor_threadlocal_t), and more. + + +Try to minimize sharing between threads: it is usually best to simply +make the worker "own" all the data it needs while the work is in +progress, and to give up ownership when it's complete. + diff --git a/src/lib/thread/threads.h b/src/lib/thread/threads.h index ecf60641b5..ead4dc3874 100644 --- a/src/lib/thread/threads.h +++ b/src/lib/thread/threads.h @@ -1,6 +1,6 @@ /* Copyright (c) 2003-2004, Roger Dingledine * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. - * Copyright (c) 2007-2019, The Tor Project, Inc. */ + * Copyright (c) 2007-2020, The Tor Project, Inc. */ /* See LICENSE for licensing information */ /** @@ -42,12 +42,7 @@ typedef struct tor_cond_t { #ifdef USE_PTHREADS pthread_cond_t cond; #elif defined(USE_WIN32_THREADS) - HANDLE event; - - CRITICAL_SECTION lock; - int n_waiting; - int n_to_wake; - int generation; + CONDITION_VARIABLE cond; #else #error no known condition implementation. #endif /* defined(USE_PTHREADS) || ... */ @@ -63,7 +58,7 @@ int tor_cond_wait(tor_cond_t *cond, tor_mutex_t *mutex, void tor_cond_signal_one(tor_cond_t *cond); void tor_cond_signal_all(tor_cond_t *cond); -typedef struct tor_threadlocal_s { +typedef struct tor_threadlocal_t { #ifdef _WIN32 DWORD index; #else @@ -106,8 +101,10 @@ void tor_threadlocal_set(tor_threadlocal_t *threadlocal, void *value); typedef struct atomic_counter_t { atomic_size_t val; } atomic_counter_t; +#ifndef COCCI #define ATOMIC_LINKAGE static -#else /* !(defined(HAVE_WORKING_STDATOMIC)) */ +#endif +#else /* !defined(HAVE_WORKING_STDATOMIC) */ typedef struct atomic_counter_t { tor_mutex_t mutex; size_t val; @@ -131,7 +128,17 @@ atomic_counter_init(atomic_counter_t *counter) { atomic_init(&counter->val, 0); } -/** Clean up all resources held by an atomic counter. */ +/** Clean up all resources held by an atomic counter. + * + * This usage note applies to the compat_threads implementation of + * atomic_counter_destroy(): + * Destroying a locked mutex is undefined behaviour. Global mutexes may be + * locked when they are passed to this function, because multiple threads can + * still access them. So we can either: + * - destroy on shutdown, and re-initialise when tor re-initialises, or + * - skip destroying and re-initialisation, using a sentinel variable. + * See #31735 for details. + */ static inline void atomic_counter_destroy(atomic_counter_t *counter) { @@ -162,7 +169,7 @@ atomic_counter_exchange(atomic_counter_t *counter, size_t newval) return atomic_exchange(&counter->val, newval); } -#else /* !(defined(HAVE_WORKING_STDATOMIC)) */ +#else /* !defined(HAVE_WORKING_STDATOMIC) */ #endif /* defined(HAVE_WORKING_STDATOMIC) */ #endif /* !defined(TOR_COMPAT_THREADS_H) */ |