From 880e3feebcc0774199fa635f1c2f7b25f99cb81f Mon Sep 17 00:00:00 2001 From: Ivan Zhakov Date: Mon, 20 May 2019 20:47:10 +0000 Subject: Use native Slim Reader/Writer (SRW) locks on Windows for apr_rwlock_t. This fixes PR 45455 and PR 51360. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1859584 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 6 ++ include/arch/win32/apr_arch_thread_rwlock.h | 5 +- locks/win32/thread_rwlock.c | 141 +++++++--------------------- 3 files changed, 44 insertions(+), 108 deletions(-) diff --git a/CHANGES b/CHANGES index 0ba7700de..7699e9119 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,12 @@ -*- coding: utf-8 -*- Changes for APR 2.0.0 + *) apr_rwlock_t: Use native Slim Reader/Writer (SRW) locks on Windows. + PR 51360. [Ivan Zhakov] + + *) Fix possible race condition in the Win32 apr_rwlock_t implementation. + PR 45455. [Ivan Zhakov] + *) apr_thread_once: Fix problem that apr_thread_once can return before the other thread completes initialization on Windows. diff --git a/include/arch/win32/apr_arch_thread_rwlock.h b/include/arch/win32/apr_arch_thread_rwlock.h index 1177e5291..7633bf56d 100644 --- a/include/arch/win32/apr_arch_thread_rwlock.h +++ b/include/arch/win32/apr_arch_thread_rwlock.h @@ -21,9 +21,8 @@ struct apr_thread_rwlock_t { apr_pool_t *pool; - HANDLE write_mutex; - HANDLE read_event; - LONG readers; + SRWLOCK lock; + int has_wrlock; }; #endif /* THREAD_RWLOCK_H */ diff --git a/locks/win32/thread_rwlock.c b/locks/win32/thread_rwlock.c index fd9d579f1..009b6fd43 100644 --- a/locks/win32/thread_rwlock.c +++ b/locks/win32/thread_rwlock.c @@ -21,104 +21,32 @@ #include "apr_arch_thread_rwlock.h" #include "apr_portable.h" -static apr_status_t thread_rwlock_cleanup(void *data) +APR_DECLARE(apr_status_t) apr_thread_rwlock_create(apr_thread_rwlock_t **rwlock_p, + apr_pool_t *pool) { - apr_thread_rwlock_t *rwlock = data; - - if (! CloseHandle(rwlock->read_event)) - return apr_get_os_error(); - - if (! CloseHandle(rwlock->write_mutex)) - return apr_get_os_error(); - - return APR_SUCCESS; -} - -APR_DECLARE(apr_status_t)apr_thread_rwlock_create(apr_thread_rwlock_t **rwlock, - apr_pool_t *pool) -{ - *rwlock = apr_palloc(pool, sizeof(**rwlock)); + apr_thread_rwlock_t* rwlock = apr_palloc(pool, sizeof(*rwlock)); - (*rwlock)->pool = pool; - (*rwlock)->readers = 0; + rwlock->pool = pool; + rwlock->has_wrlock = 0; + InitializeSRWLock(&rwlock->lock); - if (! ((*rwlock)->read_event = CreateEvent(NULL, TRUE, FALSE, NULL))) { - *rwlock = NULL; - return apr_get_os_error(); - } - - if (! ((*rwlock)->write_mutex = CreateMutex(NULL, FALSE, NULL))) { - CloseHandle((*rwlock)->read_event); - *rwlock = NULL; - return apr_get_os_error(); - } + *rwlock_p = rwlock; - apr_pool_cleanup_register(pool, *rwlock, thread_rwlock_cleanup, - apr_pool_cleanup_null); - - return APR_SUCCESS; -} - -static apr_status_t apr_thread_rwlock_rdlock_core(apr_thread_rwlock_t *rwlock, - DWORD milliseconds) -{ - DWORD code = WaitForSingleObject(rwlock->write_mutex, milliseconds); - - if (code == WAIT_FAILED || code == WAIT_TIMEOUT) - return APR_FROM_OS_ERROR(code); - - /* We've successfully acquired the writer mutex, we can't be locked - * for write, so it's OK to add the reader lock. The writer mutex - * doubles as race condition protection for the readers counter. - */ - InterlockedIncrement(&rwlock->readers); - - if (! ResetEvent(rwlock->read_event)) - return apr_get_os_error(); - - if (! ReleaseMutex(rwlock->write_mutex)) - return apr_get_os_error(); - return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_thread_rwlock_rdlock(apr_thread_rwlock_t *rwlock) { - return apr_thread_rwlock_rdlock_core(rwlock, INFINITE); -} + AcquireSRWLockShared(&rwlock->lock); -APR_DECLARE(apr_status_t) -apr_thread_rwlock_tryrdlock(apr_thread_rwlock_t *rwlock) -{ - return apr_thread_rwlock_rdlock_core(rwlock, 0); + return APR_SUCCESS; } -static apr_status_t -apr_thread_rwlock_wrlock_core(apr_thread_rwlock_t *rwlock, DWORD milliseconds) +APR_DECLARE(apr_status_t) +apr_thread_rwlock_tryrdlock(apr_thread_rwlock_t *rwlock) { - DWORD code = WaitForSingleObject(rwlock->write_mutex, milliseconds); - - if (code == WAIT_FAILED || code == WAIT_TIMEOUT) - return APR_FROM_OS_ERROR(code); - - /* We've got the writer lock but we have to wait for all readers to - * unlock before it's ok to use it. - */ - if (rwlock->readers) { - /* Must wait for readers to finish before returning, unless this - * is an trywrlock (milliseconds == 0): - */ - code = milliseconds - ? WaitForSingleObject(rwlock->read_event, milliseconds) - : WAIT_TIMEOUT; - - if (code == WAIT_FAILED || code == WAIT_TIMEOUT) { - /* Unable to wait for readers to finish, release write lock: */ - if (! ReleaseMutex(rwlock->write_mutex)) - return apr_get_os_error(); - - return APR_FROM_OS_ERROR(code); - } + if (!TryAcquireSRWLockShared(&rwlock->lock)) { + return APR_EBUSY; } return APR_SUCCESS; @@ -126,40 +54,43 @@ apr_thread_rwlock_wrlock_core(apr_thread_rwlock_t *rwlock, DWORD milliseconds) APR_DECLARE(apr_status_t) apr_thread_rwlock_wrlock(apr_thread_rwlock_t *rwlock) { - return apr_thread_rwlock_wrlock_core(rwlock, INFINITE); + AcquireSRWLockExclusive(&rwlock->lock); + rwlock->has_wrlock = 1; + + return APR_SUCCESS; } APR_DECLARE(apr_status_t)apr_thread_rwlock_trywrlock(apr_thread_rwlock_t *rwlock) { - return apr_thread_rwlock_wrlock_core(rwlock, 0); + if (!TryAcquireSRWLockExclusive(&rwlock->lock)) { + return APR_EBUSY; + } + + rwlock->has_wrlock = 1; + + return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_thread_rwlock_unlock(apr_thread_rwlock_t *rwlock) { - apr_status_t rv = 0; - - /* First, guess that we're unlocking a writer */ - if (! ReleaseMutex(rwlock->write_mutex)) - rv = apr_get_os_error(); - - if (rv == APR_FROM_OS_ERROR(ERROR_NOT_OWNER)) { - /* Nope, we must have a read lock */ - if (rwlock->readers && - ! InterlockedDecrement(&rwlock->readers) && - ! SetEvent(rwlock->read_event)) { - rv = apr_get_os_error(); - } - else { - rv = 0; - } + if (rwlock->has_wrlock) { + /* Someone holds the write lock. It MUST be the calling thread, since + the caller wants to unlock. */ + rwlock->has_wrlock = 0; + ReleaseSRWLockExclusive(&rwlock->lock); + } + else { + /* Calling thread holds the read lock. */ + ReleaseSRWLockShared(&rwlock->lock); } - return rv; + return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_thread_rwlock_destroy(apr_thread_rwlock_t *rwlock) { - return apr_pool_cleanup_run(rwlock->pool, rwlock, thread_rwlock_cleanup); + /* SRW lock is statically allocated. */ + return APR_SUCCESS; } APR_POOL_IMPLEMENT_ACCESSOR(thread_rwlock) -- cgit v1.2.1