On 'xmllite' branch: Merge changes from trunk.xmllite

git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/xmllite@1860149 13f79535-47bb-0310-9956-ffa450edef68

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 <limits.h>
@@ -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