win: fix unsavory rwlock fallback implementation

Before this patch an uv_mutex_t (backed by a critical section) could be
released by a tread different from the thread that acquired it, which is
not allowed. This is fixed by using a semaphore instead.

Note that the affected code paths were used on Windows XP and Windows
Server 2003 only.

This is a back-port of commits 3eb6764, 1ad6ad7, 9a4fd26, 9823922
85adf43 and bd1777f from the v1.x branch.

Fixes: https://github.com/libuv/libuv/issues/515
Refs: https://github.com/libuv/libuv/pull/525
PR-URL: https://github.com/libuv/libuv/pull/903
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
This commit is contained in:
Bert Belder 2015-09-10 12:51:40 +02:00 committed by Saúl Ibarra Corretgé
parent 65223248da
commit d4ff8fd5c1
7 changed files with 164 additions and 259 deletions

View File

@ -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 {

View File

@ -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;
}

View File

@ -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

View File

@ -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");

View File

@ -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;

View File

@ -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)

View File

@ -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;
}