diff --git a/include/uv-private/uv-win.h b/include/uv-private/uv-win.h index c9ec38ef..f5876d3d 100644 --- a/include/uv-private/uv-win.h +++ b/include/uv-private/uv-win.h @@ -235,14 +235,20 @@ typedef union { } uv_cond_t; typedef union { - /* srwlock_ has type SRWLOCK, but not all toolchains define this type in */ - /* windows.h. */ - SRWLOCK srwlock_; struct { - uv_mutex_t read_mutex_; - uv_mutex_t write_mutex_; unsigned int num_readers_; - } fallback_; + CRITICAL_SECTION num_readers_lock_; + HANDLE write_semaphore_; + } state_; + /* TODO: remove me in v2.x. */ + struct { + SRWLOCK unused_; + } unused1_; + /* TODO: remove me in v2.x. */ + struct { + uv_mutex_t unused1_; + uv_mutex_t unused2_; + } unused2_; } uv_rwlock_t; typedef struct { diff --git a/src/unix/thread.c b/src/unix/thread.c index 4d59e938..55b8cb7e 100644 --- a/src/unix/thread.c +++ b/src/unix/thread.c @@ -84,13 +84,12 @@ int uv_mutex_trylock(uv_mutex_t* mutex) { r = pthread_mutex_trylock(mutex); - if (r && r != EBUSY && r != EAGAIN) - abort(); - - if (r) + if (r) { + if (r != EBUSY && r != EAGAIN) + abort(); return -1; - else - return 0; + } + return 0; } @@ -125,13 +124,12 @@ int uv_rwlock_tryrdlock(uv_rwlock_t* rwlock) { r = pthread_rwlock_tryrdlock(rwlock); - if (r && r != EBUSY && r != EAGAIN) - abort(); - - if (r) + if (r) { + if (r != EBUSY && r != EAGAIN) + abort(); return -1; - else - return 0; + } + return 0; } @@ -152,13 +150,12 @@ int uv_rwlock_trywrlock(uv_rwlock_t* rwlock) { r = pthread_rwlock_trywrlock(rwlock); - if (r && r != EBUSY && r != EAGAIN) - abort(); - - if (r) + if (r) { + if (r != EBUSY && r != EAGAIN) + abort(); return -1; - else - return 0; + } + return 0; } diff --git a/src/win/thread.c b/src/win/thread.c index e774fd9c..43ee184f 100644 --- a/src/win/thread.c +++ b/src/win/thread.c @@ -26,7 +26,6 @@ #include "internal.h" -#define HAVE_SRWLOCK_API() (pTryAcquireSRWLockShared != NULL) #define HAVE_CONDVAR_API() (pInitializeConditionVariable != NULL) #ifdef _MSC_VER /* msvc */ @@ -38,25 +37,6 @@ #endif -inline static int uv__rwlock_srwlock_init(uv_rwlock_t* rwlock); -inline static void uv__rwlock_srwlock_destroy(uv_rwlock_t* rwlock); -inline static void uv__rwlock_srwlock_rdlock(uv_rwlock_t* rwlock); -inline static int uv__rwlock_srwlock_tryrdlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_srwlock_rdunlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_srwlock_wrlock(uv_rwlock_t* rwlock); -inline static int uv__rwlock_srwlock_trywrlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_srwlock_wrunlock(uv_rwlock_t* rwlock); - -inline static int uv__rwlock_fallback_init(uv_rwlock_t* rwlock); -inline static void uv__rwlock_fallback_destroy(uv_rwlock_t* rwlock); -inline static void uv__rwlock_fallback_rdlock(uv_rwlock_t* rwlock); -inline static int uv__rwlock_fallback_tryrdlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_fallback_rdunlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_fallback_wrlock(uv_rwlock_t* rwlock); -inline static int uv__rwlock_fallback_trywrlock(uv_rwlock_t* rwlock); -inline static void uv__rwlock_fallback_wrunlock(uv_rwlock_t* rwlock); - - inline static int uv_cond_fallback_init(uv_cond_t* cond); inline static void uv_cond_fallback_destroy(uv_cond_t* cond); inline static void uv_cond_fallback_signal(uv_cond_t* cond); @@ -158,68 +138,112 @@ void uv_mutex_unlock(uv_mutex_t* mutex) { int uv_rwlock_init(uv_rwlock_t* rwlock) { - uv__once_init(); + /* Initialize the semaphore that acts as the write lock. */ + HANDLE handle = CreateSemaphoreW(NULL, 1, 1, NULL); + if (handle == NULL) + return -1; + rwlock->state_.write_semaphore_ = handle; - if (HAVE_SRWLOCK_API()) - return uv__rwlock_srwlock_init(rwlock); - else - return uv__rwlock_fallback_init(rwlock); + /* Initialize the critical section protecting the reader count. */ + InitializeCriticalSection(&rwlock->state_.num_readers_lock_); + + /* Initialize the reader count. */ + rwlock->state_.num_readers_ = 0; + + return 0; } void uv_rwlock_destroy(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - uv__rwlock_srwlock_destroy(rwlock); - else - uv__rwlock_fallback_destroy(rwlock); + DeleteCriticalSection(&rwlock->state_.num_readers_lock_); + CloseHandle(rwlock->state_.write_semaphore_); } void uv_rwlock_rdlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - uv__rwlock_srwlock_rdlock(rwlock); - else - uv__rwlock_fallback_rdlock(rwlock); + /* Acquire the lock that protects the reader count. */ + EnterCriticalSection(&rwlock->state_.num_readers_lock_); + + /* Increase the reader count, and lock for write if this is the first + * reader. + */ + if (++rwlock->state_.num_readers_ == 1) { + DWORD r = WaitForSingleObject(rwlock->state_.write_semaphore_, INFINITE); + if (r != WAIT_OBJECT_0) + uv_fatal_error(GetLastError(), "WaitForSingleObject"); + } + + /* Release the lock that protects the reader count. */ + LeaveCriticalSection(&rwlock->state_.num_readers_lock_); } int uv_rwlock_tryrdlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - return uv__rwlock_srwlock_tryrdlock(rwlock); - else - return uv__rwlock_fallback_tryrdlock(rwlock); + int err; + + if (!TryEnterCriticalSection(&rwlock->state_.num_readers_lock_)) + return -1; + + err = 0; + + if (rwlock->state_.num_readers_ == 0) { + /* Currently there are no other readers, which means that the write lock + * needs to be acquired. + */ + DWORD r = WaitForSingleObject(rwlock->state_.write_semaphore_, 0); + if (r == WAIT_OBJECT_0) + rwlock->state_.num_readers_++; + else if (r == WAIT_TIMEOUT) + err = -1; + else if (r == WAIT_FAILED) + uv_fatal_error(GetLastError(), "WaitForSingleObject"); + + } else { + /* The write lock has already been acquired because there are other + * active readers. + */ + rwlock->state_.num_readers_++; + } + + LeaveCriticalSection(&rwlock->state_.num_readers_lock_); + return err; } void uv_rwlock_rdunlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - uv__rwlock_srwlock_rdunlock(rwlock); - else - uv__rwlock_fallback_rdunlock(rwlock); + EnterCriticalSection(&rwlock->state_.num_readers_lock_); + + if (--rwlock->state_.num_readers_ == 0) { + if (!ReleaseSemaphore(rwlock->state_.write_semaphore_, 1, NULL)) + uv_fatal_error(GetLastError(), "ReleaseSemaphore"); + } + + LeaveCriticalSection(&rwlock->state_.num_readers_lock_); } void uv_rwlock_wrlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - uv__rwlock_srwlock_wrlock(rwlock); - else - uv__rwlock_fallback_wrlock(rwlock); + DWORD r = WaitForSingleObject(rwlock->state_.write_semaphore_, INFINITE); + if (r != WAIT_OBJECT_0) + uv_fatal_error(GetLastError(), "WaitForSingleObject"); } int uv_rwlock_trywrlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - return uv__rwlock_srwlock_trywrlock(rwlock); + DWORD r = WaitForSingleObject(rwlock->state_.write_semaphore_, 0); + if (r == WAIT_OBJECT_0) + return 0; + else if (r == WAIT_TIMEOUT) + return -1; else - return uv__rwlock_fallback_trywrlock(rwlock); + uv_fatal_error(GetLastError(), "WaitForSingleObject"); + return -1; } void uv_rwlock_wrunlock(uv_rwlock_t* rwlock) { - if (HAVE_SRWLOCK_API()) - uv__rwlock_srwlock_wrunlock(rwlock); - else - uv__rwlock_fallback_wrunlock(rwlock); + if (!ReleaseSemaphore(rwlock->state_.write_semaphore_, 1, NULL)) + uv_fatal_error(GetLastError(), "ReleaseSemaphore"); } @@ -261,133 +285,6 @@ int uv_sem_trywait(uv_sem_t* sem) { } -inline static int uv__rwlock_srwlock_init(uv_rwlock_t* rwlock) { - pInitializeSRWLock(&rwlock->srwlock_); - return 0; -} - - -inline static void uv__rwlock_srwlock_destroy(uv_rwlock_t* rwlock) { - (void) rwlock; -} - - -inline static void uv__rwlock_srwlock_rdlock(uv_rwlock_t* rwlock) { - pAcquireSRWLockShared(&rwlock->srwlock_); -} - - -inline static int uv__rwlock_srwlock_tryrdlock(uv_rwlock_t* rwlock) { - if (pTryAcquireSRWLockShared(&rwlock->srwlock_)) - return 0; - else - return -1; -} - - -inline static void uv__rwlock_srwlock_rdunlock(uv_rwlock_t* rwlock) { - pReleaseSRWLockShared(&rwlock->srwlock_); -} - - -inline static void uv__rwlock_srwlock_wrlock(uv_rwlock_t* rwlock) { - pAcquireSRWLockExclusive(&rwlock->srwlock_); -} - - -inline static int uv__rwlock_srwlock_trywrlock(uv_rwlock_t* rwlock) { - if (pTryAcquireSRWLockExclusive(&rwlock->srwlock_)) - return 0; - else - return -1; -} - - -inline static void uv__rwlock_srwlock_wrunlock(uv_rwlock_t* rwlock) { - pReleaseSRWLockExclusive(&rwlock->srwlock_); -} - - -inline static int uv__rwlock_fallback_init(uv_rwlock_t* rwlock) { - if (uv_mutex_init(&rwlock->fallback_.read_mutex_)) - return -1; - - if (uv_mutex_init(&rwlock->fallback_.write_mutex_)) { - uv_mutex_destroy(&rwlock->fallback_.read_mutex_); - return -1; - } - - rwlock->fallback_.num_readers_ = 0; - - return 0; -} - - -inline static void uv__rwlock_fallback_destroy(uv_rwlock_t* rwlock) { - uv_mutex_destroy(&rwlock->fallback_.read_mutex_); - uv_mutex_destroy(&rwlock->fallback_.write_mutex_); -} - - -inline static void uv__rwlock_fallback_rdlock(uv_rwlock_t* rwlock) { - uv_mutex_lock(&rwlock->fallback_.read_mutex_); - - if (++rwlock->fallback_.num_readers_ == 1) - uv_mutex_lock(&rwlock->fallback_.write_mutex_); - - uv_mutex_unlock(&rwlock->fallback_.read_mutex_); -} - - -inline static int uv__rwlock_fallback_tryrdlock(uv_rwlock_t* rwlock) { - int ret; - - ret = -1; - - if (uv_mutex_trylock(&rwlock->fallback_.read_mutex_)) - goto out; - - if (rwlock->fallback_.num_readers_ == 0) - ret = uv_mutex_trylock(&rwlock->fallback_.write_mutex_); - else - ret = 0; - - if (ret == 0) - rwlock->fallback_.num_readers_++; - - uv_mutex_unlock(&rwlock->fallback_.read_mutex_); - -out: - return ret; -} - - -inline static void uv__rwlock_fallback_rdunlock(uv_rwlock_t* rwlock) { - uv_mutex_lock(&rwlock->fallback_.read_mutex_); - - if (--rwlock->fallback_.num_readers_ == 0) - uv_mutex_unlock(&rwlock->fallback_.write_mutex_); - - uv_mutex_unlock(&rwlock->fallback_.read_mutex_); -} - - -inline static void uv__rwlock_fallback_wrlock(uv_rwlock_t* rwlock) { - uv_mutex_lock(&rwlock->fallback_.write_mutex_); -} - - -inline static int uv__rwlock_fallback_trywrlock(uv_rwlock_t* rwlock) { - return uv_mutex_trylock(&rwlock->fallback_.write_mutex_); -} - - -inline static void uv__rwlock_fallback_wrunlock(uv_rwlock_t* rwlock) { - uv_mutex_unlock(&rwlock->fallback_.write_mutex_); -} - - - /* This condition variable implementation is based on the SetEvent solution * (section 3.2) at http://www.cs.wustl.edu/~schmidt/win32-cv-1.html * We could not use the SignalObjectAndWait solution (section 3.4) because diff --git a/src/win/winapi.c b/src/win/winapi.c index ab68bba7..1f067736 100644 --- a/src/win/winapi.c +++ b/src/win/winapi.c @@ -38,13 +38,6 @@ sGetQueuedCompletionStatusEx pGetQueuedCompletionStatusEx; sSetFileCompletionNotificationModes pSetFileCompletionNotificationModes; sCreateSymbolicLinkW pCreateSymbolicLinkW; sCancelIoEx pCancelIoEx; -sInitializeSRWLock pInitializeSRWLock; -sAcquireSRWLockShared pAcquireSRWLockShared; -sAcquireSRWLockExclusive pAcquireSRWLockExclusive; -sTryAcquireSRWLockShared pTryAcquireSRWLockShared; -sTryAcquireSRWLockExclusive pTryAcquireSRWLockExclusive; -sReleaseSRWLockShared pReleaseSRWLockShared; -sReleaseSRWLockExclusive pReleaseSRWLockExclusive; sInitializeConditionVariable pInitializeConditionVariable; sSleepConditionVariableCS pSleepConditionVariableCS; sSleepConditionVariableSRW pSleepConditionVariableSRW; @@ -114,27 +107,6 @@ void uv_winapi_init() { pCancelIoEx = (sCancelIoEx) GetProcAddress(kernel32_module, "CancelIoEx"); - pInitializeSRWLock = (sInitializeSRWLock) - GetProcAddress(kernel32_module, "InitializeSRWLock"); - - pAcquireSRWLockShared = (sAcquireSRWLockShared) - GetProcAddress(kernel32_module, "AcquireSRWLockShared"); - - pAcquireSRWLockExclusive = (sAcquireSRWLockExclusive) - GetProcAddress(kernel32_module, "AcquireSRWLockExclusive"); - - pTryAcquireSRWLockShared = (sTryAcquireSRWLockShared) - GetProcAddress(kernel32_module, "TryAcquireSRWLockShared"); - - pTryAcquireSRWLockExclusive = (sTryAcquireSRWLockExclusive) - GetProcAddress(kernel32_module, "TryAcquireSRWLockExclusive"); - - pReleaseSRWLockShared = (sReleaseSRWLockShared) - GetProcAddress(kernel32_module, "ReleaseSRWLockShared"); - - pReleaseSRWLockExclusive = (sReleaseSRWLockExclusive) - GetProcAddress(kernel32_module, "ReleaseSRWLockExclusive"); - pInitializeConditionVariable = (sInitializeConditionVariable) GetProcAddress(kernel32_module, "InitializeConditionVariable"); diff --git a/src/win/winapi.h b/src/win/winapi.h index e023beed..9e78b6b8 100644 --- a/src/win/winapi.h +++ b/src/win/winapi.h @@ -4405,27 +4405,6 @@ typedef BOOL (WINAPI* sCancelIoEx) (HANDLE hFile, LPOVERLAPPED lpOverlapped); -typedef VOID (WINAPI* sInitializeSRWLock) - (PSRWLOCK SRWLock); - -typedef VOID (WINAPI* sAcquireSRWLockShared) - (PSRWLOCK SRWLock); - -typedef VOID (WINAPI* sAcquireSRWLockExclusive) - (PSRWLOCK SRWLock); - -typedef BOOL (WINAPI* sTryAcquireSRWLockShared) - (PSRWLOCK SRWLock); - -typedef BOOL (WINAPI* sTryAcquireSRWLockExclusive) - (PSRWLOCK SRWLock); - -typedef VOID (WINAPI* sReleaseSRWLockShared) - (PSRWLOCK SRWLock); - -typedef VOID (WINAPI* sReleaseSRWLockExclusive) - (PSRWLOCK SRWLock); - typedef VOID (WINAPI* sInitializeConditionVariable) (PCONDITION_VARIABLE ConditionVariable); @@ -4460,13 +4439,6 @@ extern sGetQueuedCompletionStatusEx pGetQueuedCompletionStatusEx; extern sSetFileCompletionNotificationModes pSetFileCompletionNotificationModes; extern sCreateSymbolicLinkW pCreateSymbolicLinkW; extern sCancelIoEx pCancelIoEx; -extern sInitializeSRWLock pInitializeSRWLock; -extern sAcquireSRWLockShared pAcquireSRWLockShared; -extern sAcquireSRWLockExclusive pAcquireSRWLockExclusive; -extern sTryAcquireSRWLockShared pTryAcquireSRWLockShared; -extern sTryAcquireSRWLockExclusive pTryAcquireSRWLockExclusive; -extern sReleaseSRWLockShared pReleaseSRWLockShared; -extern sReleaseSRWLockExclusive pReleaseSRWLockExclusive; extern sInitializeConditionVariable pInitializeConditionVariable; extern sSleepConditionVariableCS pSleepConditionVariableCS; extern sSleepConditionVariableSRW pSleepConditionVariableSRW; diff --git a/test/test-list.h b/test/test-list.h index 0bfb9e41..51c9fe32 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -211,6 +211,7 @@ TEST_DECLARE (threadpool_cancel_fs) TEST_DECLARE (threadpool_cancel_single) TEST_DECLARE (thread_mutex) TEST_DECLARE (thread_rwlock) +TEST_DECLARE (thread_rwlock_trylock) TEST_DECLARE (thread_create) TEST_DECLARE (strlcpy) TEST_DECLARE (strlcat) @@ -522,6 +523,7 @@ TASK_LIST_START TEST_ENTRY (threadpool_cancel_single) TEST_ENTRY (thread_mutex) TEST_ENTRY (thread_rwlock) + TEST_ENTRY (thread_rwlock_trylock) TEST_ENTRY (thread_create) TEST_ENTRY (strlcpy) TEST_ENTRY (strlcat) diff --git a/test/test-mutexes.c b/test/test-mutexes.c index 896f46bb..5f8bc46f 100644 --- a/test/test-mutexes.c +++ b/test/test-mutexes.c @@ -61,3 +61,62 @@ TEST_IMPL(thread_rwlock) { return 0; } + + +TEST_IMPL(thread_rwlock_trylock) { + uv_rwlock_t rwlock; + int r; + + r = uv_rwlock_init(&rwlock); + ASSERT(r == 0); + + /* No locks held. */ + + r = uv_rwlock_trywrlock(&rwlock); + ASSERT(r == 0); + + /* Write lock held. */ + + r = uv_rwlock_tryrdlock(&rwlock); + ASSERT(r == -1); + r = uv_rwlock_trywrlock(&rwlock); + ASSERT(r == -1); + + uv_rwlock_wrunlock(&rwlock); + + /* No locks held. */ + + r = uv_rwlock_tryrdlock(&rwlock); + ASSERT(r == 0); + + /* One read lock held. */ + + r = uv_rwlock_tryrdlock(&rwlock); + ASSERT(r == 0); + + /* Two read locks held. */ + + r = uv_rwlock_trywrlock(&rwlock); + ASSERT(r == -1); + + uv_rwlock_rdunlock(&rwlock); + + /* One read lock held. */ + + uv_rwlock_rdunlock(&rwlock); + + /* No read locks held. */ + + r = uv_rwlock_trywrlock(&rwlock); + ASSERT(r == 0); + + /* Write lock held. */ + + uv_rwlock_wrunlock(&rwlock); + + /* No locks held. */ + + uv_rwlock_destroy(&rwlock); + + return 0; +}