commit 877dbfc0561dfc5297cf446fe653cc3095d20b46 Author: Daniel Pinto <danielpinto52@gmail.com> Date: Thu Nov 12 19:40:07 2020 +0000 Use SRWLocks to implement locking on Windows #17927 Replace the Windows locking implementation which used critical sections with the faster SRWLocks available since Vista. --- changes/ticket17927 | 4 ++ src/lib/lock/compat_mutex.h | 11 ++++- src/lib/lock/compat_mutex_winthreads.c | 76 +++++++++++++++++++++++++++++++--- src/lib/thread/compat_winthreads.c | 8 +++- 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/changes/ticket17927 b/changes/ticket17927 new file mode 100644 index 0000000000..532416dac4 --- /dev/null +++ b/changes/ticket17927 @@ -0,0 +1,4 @@ + o Minor features (performance, windows): + - Use SRWLocks to implement locking on Windows. Replaces the critical + section locking implementation with the faster SRWLocks available + since Windows Vista. Closes ticket 17927. Patch by Daniel Pinto. diff --git a/src/lib/lock/compat_mutex.h b/src/lib/lock/compat_mutex.h index 5631993cc4..14bdf70ce5 100644 --- a/src/lib/lock/compat_mutex.h +++ b/src/lib/lock/compat_mutex.h @@ -39,8 +39,15 @@ /** A generic lock structure for multithreaded builds. */ typedef struct tor_mutex_t { #if defined(USE_WIN32_THREADS) - /** Windows-only: on windows, we implement locks with CRITICAL_SECTIONS. */ - CRITICAL_SECTION mutex; + /** Windows-only: on windows, we implement locks with SRW locks. */ + SRWLOCK mutex; + /** For recursive lock support (SRW locks are not recursive) */ + enum mutex_type_t { + NON_RECURSIVE = 0, + RECURSIVE + } type; + DWORD lock_owner; // id of the thread that owns the lock + int lock_count; // number of times the lock is held recursively #elif defined(USE_PTHREADS) /** Pthreads-only: with pthreads, we implement locks with * pthread_mutex_t. */ diff --git a/src/lib/lock/compat_mutex_winthreads.c b/src/lib/lock/compat_mutex_winthreads.c index 5fe6870a93..03cf5082e0 100644 --- a/src/lib/lock/compat_mutex_winthreads.c +++ b/src/lib/lock/compat_mutex_winthreads.c @@ -9,6 +9,23 @@ * \brief Implement the tor_mutex API using CRITICAL_SECTION. **/ +#include "orconfig.h" + +/* For SRW locks 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 "lib/lock/compat_mutex.h" #include "lib/err/torerr.h" @@ -20,27 +37,76 @@ tor_locking_init(void) void tor_mutex_init(tor_mutex_t *m) { - InitializeCriticalSection(&m->mutex); + m->type = RECURSIVE; + m->lock_owner = 0; + m->lock_count = 0; + InitializeSRWLock(&m->mutex); } void tor_mutex_init_nonrecursive(tor_mutex_t *m) { - InitializeCriticalSection(&m->mutex); + m->type = NON_RECURSIVE; + InitializeSRWLock(&m->mutex); } void tor_mutex_uninit(tor_mutex_t *m) { - DeleteCriticalSection(&m->mutex); + (void) m; +} + +static void +tor_mutex_acquire_recursive(tor_mutex_t *m) +{ + DWORD thread_id = GetCurrentThreadId(); + if (thread_id == m->lock_owner) { + ++m->lock_count; + return; + } + AcquireSRWLockExclusive(&m->mutex); + m->lock_owner = thread_id; + m->lock_count = 1; +} + +static void +tor_mutex_acquire_nonrecursive(tor_mutex_t *m) +{ + AcquireSRWLockExclusive(&m->mutex); } + void tor_mutex_acquire(tor_mutex_t *m) { raw_assert(m); - EnterCriticalSection(&m->mutex); + if (m->type == NON_RECURSIVE) { + tor_mutex_acquire_nonrecursive(m); + } else { + tor_mutex_acquire_recursive(m); + } +} + +static void +tor_mutex_release_recursive(tor_mutex_t *m) +{ + if (--m->lock_count) { + return; + } + m->lock_owner = 0; + ReleaseSRWLockExclusive(&m->mutex); } + +static void +tor_mutex_release_nonrecursive(tor_mutex_t *m) +{ + ReleaseSRWLockExclusive(&m->mutex); +} + void tor_mutex_release(tor_mutex_t *m) { - LeaveCriticalSection(&m->mutex); + if (m->type == NON_RECURSIVE) { + tor_mutex_release_nonrecursive(m); + } else { + tor_mutex_release_recursive(m); + } } diff --git a/src/lib/thread/compat_winthreads.c b/src/lib/thread/compat_winthreads.c index fcc9c0279b..a6213aa46a 100644 --- a/src/lib/thread/compat_winthreads.c +++ b/src/lib/thread/compat_winthreads.c @@ -144,13 +144,17 @@ tor_threadlocal_set(tor_threadlocal_t *threadlocal, void *value) int tor_cond_wait(tor_cond_t *cond, tor_mutex_t *lock_, const struct timeval *tv) { - CRITICAL_SECTION *lock = &lock_->mutex; + // recursive SRW locks are not supported because they need extra logic for + // acquiring and releasing but SleepConditionVariableSRW will use the OS + // lock relase function which lacks our extra logic + tor_assert(lock_->type == NON_RECURSIVE); + SRWLOCK *lock = &lock_->mutex; DWORD ms = INFINITE; if (tv) { ms = tv->tv_sec*1000 + (tv->tv_usec+999)/1000; } - BOOL ok = SleepConditionVariableCS(&cond->cond, lock, ms); + BOOL ok = SleepConditionVariableSRW(&cond->cond, lock, ms, 0); if (!ok) { DWORD err = GetLastError(); if (err == ERROR_TIMEOUT) {