From 5a0567d0470e8be395c0299306786a98992c6f25 Mon Sep 17 00:00:00 2001 From: ivan Date: Mon, 27 May 2019 17:11:23 +0000 Subject: On 'xmllite' branch: Merge changes from trunk. git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/xmllite@1860149 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 ++ file_io/win32/open.c | 7 ++--- file_io/win32/pipe.c | 4 +-- file_io/win32/readwrite.c | 60 +++++++++++++++++++++----------------- include/arch/win32/apr_arch_misc.h | 5 ++++ locks/win32/proc_mutex.c | 23 ++------------- locks/win32/thread_cond.c | 24 ++------------- locks/win32/thread_mutex.c | 23 ++------------- misc/win32/misc.c | 30 +++++++++++++++++++ user/win32/userinfo.c | 27 ++++++++++++----- 10 files changed, 102 insertions(+), 104 deletions(-) diff --git a/CHANGES b/CHANGES index ec014be96..f46f91924 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,9 @@ Changes for APR 2.0.0 *) Windows platform: Use the native XML parser implementation (XmlLite) by default instead of libxml2 and expat. [Ivan Zhakov] + *) Fix handle leak in the Win32 apr_uid_current implementation. + PR 61165. [Ivan Zhakov] + *) apr_rwlock_t: Use native Slim Reader/Writer (SRW) locks on Windows. PR 51360. [Ivan Zhakov] diff --git a/file_io/win32/open.c b/file_io/win32/open.c index f3b93f5ae..fca4397a8 100644 --- a/file_io/win32/open.c +++ b/file_io/win32/open.c @@ -252,11 +252,8 @@ static apr_status_t make_sparse_file(apr_file_t *file) if (rv == APR_FROM_OS_ERROR(ERROR_IO_PENDING)) { do { - res = WaitForSingleObject(file->pOverlapped->hEvent, - (file->timeout > 0) - ? (DWORD)(file->timeout/1000) - : ((file->timeout == -1) - ? INFINITE : 0)); + res = apr_wait_for_single_object(file->pOverlapped->hEvent, + file->timeout); } while (res == WAIT_ABANDONED); if (res != WAIT_OBJECT_0) { diff --git a/file_io/win32/pipe.c b/file_io/win32/pipe.c index b13507a91..8de7822af 100644 --- a/file_io/win32/pipe.c +++ b/file_io/win32/pipe.c @@ -146,7 +146,7 @@ static apr_status_t file_pipe_create(apr_file_t **in, dwOpenMode |= FILE_FLAG_OVERLAPPED; (*in)->pOverlapped = (OVERLAPPED*) apr_pcalloc((*in)->pool, sizeof(OVERLAPPED)); - (*in)->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + (*in)->pOverlapped->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); (*in)->timeout = 0; } dwPipeMode = 0; @@ -181,7 +181,7 @@ static apr_status_t file_pipe_create(apr_file_t **in, dwOpenMode |= FILE_FLAG_OVERLAPPED; (*out)->pOverlapped = (OVERLAPPED*) apr_pcalloc((*out)->pool, sizeof(OVERLAPPED)); - (*out)->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + (*out)->pOverlapped->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); (*out)->timeout = 0; } diff --git a/file_io/win32/readwrite.c b/file_io/win32/readwrite.c index bf7cbfb30..23d2358e7 100644 --- a/file_io/win32/readwrite.c +++ b/file_io/win32/readwrite.c @@ -31,7 +31,6 @@ static apr_status_t read_with_timeout(apr_file_t *file, void *buf, apr_size_t len_in, apr_size_t *nbytes) { apr_status_t rv; - DWORD res; DWORD len = (DWORD)len_in; DWORD bytesread = 0; @@ -80,19 +79,26 @@ static apr_status_t read_with_timeout(apr_file_t *file, void *buf, apr_size_t le else { rv = apr_get_os_error(); if (rv == APR_FROM_OS_ERROR(ERROR_IO_PENDING)) { - /* Wait for the pending i/o, timeout converted from us to ms - * Note that we loop if someone gives up the event, since - * folks suggest that WAIT_ABANDONED isn't actually a result - * but an alert that ownership of the event has passed from - * one owner to a new proc/thread. - */ - do { - res = WaitForSingleObject(file->pOverlapped->hEvent, - (file->timeout > 0) - ? (DWORD)(file->timeout/1000) - : ((file->timeout == -1) - ? INFINITE : 0)); - } while (res == WAIT_ABANDONED); + DWORD res; + + /* It seems that ReadFile() return ERROR_IO_PENDING even + * when I/O operation completed syncronously. + * Use fast macro to check that overlapped I/O already + * completed to avoid kernel call. + */ + if (HasOverlappedIoCompleted(file->pOverlapped)) { + res = WAIT_OBJECT_0; + } + else { + /* Wait for the pending i/o, timeout converted from us to ms + * Note that we loop if someone gives up the event. + * + * NOTE: We do not handle WAIT_ABANDONED here because they + * can be returned only when waiting for mutex. + */ + res = apr_wait_for_single_object(file->pOverlapped->hEvent, + file->timeout); + } /* There is one case that represents entirely * successful operations, otherwise we will cancel @@ -241,7 +247,7 @@ APR_DECLARE(apr_status_t) apr_file_read(apr_file_t *thefile, void *buf, apr_size if ((thefile->flags & APR_FOPEN_XTHREAD) && !thefile->pOverlapped ) { thefile->pOverlapped = (OVERLAPPED*) apr_pcalloc(thefile->pool, sizeof(OVERLAPPED)); - thefile->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + thefile->pOverlapped->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); if (!thefile->pOverlapped->hEvent) { rv = apr_get_os_error(); return rv; @@ -388,7 +394,7 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, a if ((thefile->flags & APR_FOPEN_XTHREAD) && !thefile->pOverlapped ) { thefile->pOverlapped = (OVERLAPPED*) apr_pcalloc(thefile->pool, sizeof(OVERLAPPED)); - thefile->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + thefile->pOverlapped->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); if (!thefile->pOverlapped->hEvent) { rv = apr_get_os_error(); return rv; @@ -492,20 +498,20 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, a if (rv == APR_FROM_OS_ERROR(ERROR_IO_PENDING)) { - DWORD timeout_ms; DWORD res; - if (thefile->timeout == 0) { - timeout_ms = 0; - } - else if (thefile->timeout < 0) { - timeout_ms = INFINITE; + /* It seems that WriteFile() return ERROR_IO_PENDING even + * when I/O operation completed syncronously. + * Use fast macro to check that overlapped I/O already + * completed to avoid kernel call. + */ + if (HasOverlappedIoCompleted(thefile->pOverlapped)) { + res = WAIT_OBJECT_0; } else { - timeout_ms = (DWORD)(thefile->timeout / 1000); + res = apr_wait_for_single_object(thefile->pOverlapped->hEvent, + thefile->timeout); } - - res = WaitForSingleObject(thefile->pOverlapped->hEvent, timeout_ms); /* There is one case that represents entirely * successful operations, otherwise we will cancel @@ -533,7 +539,7 @@ APR_DECLARE(apr_status_t) apr_file_write(apr_file_t *thefile, const void *buf, a && (res == WAIT_TIMEOUT)) rv = APR_TIMEUP; - if (rv == APR_TIMEUP && timeout_ms == 0) { + if (rv == APR_TIMEUP && thefile->timeout == 0) { rv = APR_EAGAIN; } } @@ -624,7 +630,7 @@ APR_DECLARE(apr_status_t) apr_file_gets(char *str, int len, apr_file_t *thefile) if ((thefile->flags & APR_FOPEN_XTHREAD) && !thefile->pOverlapped) { thefile->pOverlapped = (OVERLAPPED*) apr_pcalloc(thefile->pool, sizeof(OVERLAPPED)); - thefile->pOverlapped->hEvent = CreateEvent(NULL, FALSE, FALSE, NULL); + thefile->pOverlapped->hEvent = CreateEvent(NULL, TRUE, FALSE, NULL); if (!thefile->pOverlapped->hEvent) { rv = apr_get_os_error(); return rv; diff --git a/include/arch/win32/apr_arch_misc.h b/include/arch/win32/apr_arch_misc.h index 2cf1c6545..4fbe79ff4 100644 --- a/include/arch/win32/apr_arch_misc.h +++ b/include/arch/win32/apr_arch_misc.h @@ -172,6 +172,11 @@ static APR_INLINE void* apr_realloc_dbg(void* userData, size_t newSize, #endif /* ! _MSC_VER */ +/* Wrapper around WaitForSingleObject() that accepts apr_interval_time_t + * in microseconds instead of milliseconds. Values < 0 mean wait + * forever, 0 means do not wait at all. */ +DWORD apr_wait_for_single_object(HANDLE handle, apr_interval_time_t timeout); + typedef enum { DLL_WINBASEAPI = 0, /* kernel32 From WinBase.h */ DLL_WINADVAPI = 1, /* advapi32 From WinBase.h */ diff --git a/locks/win32/proc_mutex.c b/locks/win32/proc_mutex.c index e132e20a2..c43377e74 100644 --- a/locks/win32/proc_mutex.c +++ b/locks/win32/proc_mutex.c @@ -167,26 +167,9 @@ APR_DECLARE(apr_status_t) apr_proc_mutex_trylock(apr_proc_mutex_t *mutex) APR_DECLARE(apr_status_t) apr_proc_mutex_timedlock(apr_proc_mutex_t *mutex, apr_interval_time_t timeout) { - DWORD rv, timeout_ms = 0; - apr_interval_time_t t = timeout; - - do { - if (t > 0) { - /* Given timeout is 64bit usecs whereas Windows timeouts are - * 32bit msecs and below INFINITE (2^32 - 1), so we may need - * multiple timed out waits... - */ - if (t > apr_time_from_msec(INFINITE - 1)) { - timeout_ms = INFINITE - 1; - t -= apr_time_from_msec(INFINITE - 1); - } - else { - timeout_ms = (DWORD)apr_time_as_msec(t); - t = 0; - } - } - rv = WaitForSingleObject(mutex->handle, timeout_ms); - } while (rv == WAIT_TIMEOUT && t > 0); + DWORD rv; + + rv = apr_wait_for_single_object(mutex->handle, timeout); if (rv == WAIT_TIMEOUT) { return APR_TIMEUP; diff --git a/locks/win32/thread_cond.c b/locks/win32/thread_cond.c index f571f9ec9..31a46d54e 100644 --- a/locks/win32/thread_cond.c +++ b/locks/win32/thread_cond.c @@ -20,6 +20,7 @@ #include "apr_strings.h" #include "apr_arch_thread_mutex.h" #include "apr_arch_thread_cond.h" +#include "apr_arch_misc.h" #include "apr_portable.h" #include @@ -79,28 +80,7 @@ static APR_INLINE apr_status_t thread_cond_timedwait(apr_thread_cond_t *cond, apr_thread_mutex_unlock(mutex); do { - apr_interval_time_t t = timeout; - - do { - if (t < 0) { - timeout_ms = INFINITE; - } - else if (t > 0) { - /* Given timeout is 64bit usecs whereas Windows timeouts are - * 32bit msecs and below INFINITE (2^32 - 1), so we may need - * multiple timed out waits... - */ - if (t > apr_time_from_msec(INFINITE - 1)) { - timeout_ms = INFINITE - 1; - t -= apr_time_from_msec(INFINITE - 1); - } - else { - timeout_ms = (DWORD)apr_time_as_msec(t); - t = 0; - } - } - res = WaitForSingleObject(cond->semaphore, timeout_ms); - } while (res == WAIT_TIMEOUT && t > 0); + res = apr_wait_for_single_object(cond->semaphore, timeout); EnterCriticalSection(&cond->csection); diff --git a/locks/win32/thread_mutex.c b/locks/win32/thread_mutex.c index f19152495..165e34c22 100644 --- a/locks/win32/thread_mutex.c +++ b/locks/win32/thread_mutex.c @@ -118,26 +118,9 @@ APR_DECLARE(apr_status_t) apr_thread_mutex_timedlock(apr_thread_mutex_t *mutex, apr_interval_time_t timeout) { if (mutex->type != thread_mutex_critical_section) { - DWORD rv, timeout_ms = 0; - apr_interval_time_t t = timeout; - - do { - if (t > 0) { - /* Given timeout is 64bit usecs whereas Windows timeouts are - * 32bit msecs and below INFINITE (2^32 - 1), so we may need - * multiple timed out waits... - */ - if (t > apr_time_from_msec(INFINITE - 1)) { - timeout_ms = INFINITE - 1; - t -= apr_time_from_msec(INFINITE - 1); - } - else { - timeout_ms = (DWORD)apr_time_as_msec(t); - t = 0; - } - } - rv = WaitForSingleObject(mutex->handle, timeout_ms); - } while (rv == WAIT_TIMEOUT && t > 0); + DWORD rv; + + rv = apr_wait_for_single_object(mutex->handle, timeout); if ((rv != WAIT_OBJECT_0) && (rv != WAIT_ABANDONED)) { return (rv == WAIT_TIMEOUT) ? APR_TIMEUP : apr_get_os_error(); diff --git a/misc/win32/misc.c b/misc/win32/misc.c index b6ee202b0..11b5b6b18 100644 --- a/misc/win32/misc.c +++ b/misc/win32/misc.c @@ -188,6 +188,36 @@ FARPROC apr_load_dll_func(apr_dlltoken_e fnLib, char* fnName, int ordinal) #endif } +DWORD apr_wait_for_single_object(HANDLE handle, apr_interval_time_t timeout) +{ + apr_interval_time_t t = timeout; + DWORD res; + DWORD timeout_ms = 0; + + do { + if (t < 0) { + timeout_ms = INFINITE; + } + else if (t > 0) { + /* Given timeout is 64bit usecs whereas Windows timeouts are + * 32bit msecs and below INFINITE (2^32 - 1), so we may need + * multiple timed out waits... + */ + if (t > apr_time_from_msec(INFINITE - 1)) { + timeout_ms = INFINITE - 1; + t -= apr_time_from_msec(INFINITE - 1); + } + else { + timeout_ms = (DWORD)apr_time_as_msec(t); + t = 0; + } + } + res = WaitForSingleObject(handle, timeout_ms); + } while (res == WAIT_TIMEOUT && t > 0); + + return res; +} + /* Declared in include/arch/win32/apr_dbg_win32_handles.h */ APR_DECLARE_NONSTD(HANDLE) apr_dbg_log(char* fn, HANDLE ha, char* fl, int ln, diff --git a/user/win32/userinfo.c b/user/win32/userinfo.c index c6b5084a5..89f9b8cf0 100644 --- a/user/win32/userinfo.c +++ b/user/win32/userinfo.c @@ -171,27 +171,38 @@ APR_DECLARE(apr_status_t) apr_uid_current(apr_uid_t *uid, DWORD needed; TOKEN_USER *usr; TOKEN_PRIMARY_GROUP *grp; - + apr_status_t rv; + if(!OpenProcessToken(GetCurrentProcess(), STANDARD_RIGHTS_READ | READ_CONTROL | TOKEN_QUERY, &threadtok)) { return apr_get_os_error(); } *uid = NULL; if (!GetTokenInformation(threadtok, TokenUser, NULL, 0, &needed) - && (GetLastError() == ERROR_INSUFFICIENT_BUFFER) + && (GetLastError() == ERROR_INSUFFICIENT_BUFFER) && (usr = apr_palloc(p, needed)) - && GetTokenInformation(threadtok, TokenUser, usr, needed, &needed)) + && GetTokenInformation(threadtok, TokenUser, usr, needed, &needed)) { *uid = usr->User.Sid; - else - return apr_get_os_error(); + } + else { + rv = apr_get_os_error(); + CloseHandle(threadtok); + return rv; + } if (!GetTokenInformation(threadtok, TokenPrimaryGroup, NULL, 0, &needed) && (GetLastError() == ERROR_INSUFFICIENT_BUFFER) && (grp = apr_palloc(p, needed)) - && GetTokenInformation(threadtok, TokenPrimaryGroup, grp, needed, &needed)) + && GetTokenInformation(threadtok, TokenPrimaryGroup, grp, needed, &needed)) { *gid = grp->PrimaryGroup; - else - return apr_get_os_error(); + } + else { + rv = apr_get_os_error(); + CloseHandle(threadtok); + return rv; + } + + CloseHandle(threadtok); return APR_SUCCESS; #endif -- cgit v1.2.1