Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <92322E4B3209D511A19100508B55847802EC0E71@exchange.olf.com>
From: Ed Holyat <eholyat@OLF.COM>
To: "'dev@apr.apache.org'" <dev@apr.apache.org>
Subject: Patch for Win32 thread deadlock
Date: Fri, 16 Jul 2004 17:40:10 -0400
MIME-Version: 1.0
Content-Type: multipart/mixed;
	boundary="----_=_NextPart_000_01C46B7D.785C5EA0"

This message is in MIME format. Since your mail reader does not understand
this format, some or all of this message may not be legible.

------_=_NextPart_000_01C46B7D.785C5EA0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable


	Patch for Win32 thread deadlock, tested only on multi-processor XP.
The problem occurs if the thread calls apr_thread_exit() thus closing =
the
shared handle; and then the main thread tries to join the closed thread =
with
a bad handle or has already joined the thread. =20

	You can not close a handle while there is a process using that
handle in WaitForSingleObject(). =20
		"If this handle is closed while the wait is still pending,
the function's behavior is undefined. "


 <<win32_thread.diff>>=20


=A9 2004 OpenLink Financial=20

Copyright in this message and any attachments remains with us.  It is
confidential and may be legally privileged.   If this message is not=20
intended for you it must not be read, copied or used by you or=20
disclosed to anyone else.   Please advise the sender immediately if=20
you have received this message in error.

Although this message and any attachments are believed to be free of=20
any virus or other defect that might affect any computer system into=20
which it is received and opened, it is the responsibility of the=20
recipient to ensure that it is virus free and no responsibility=20
is accepted by Open Link Financial, Inc. for any loss or damage in any=20
way arising from its use.


------_=_NextPart_000_01C46B7D.785C5EA0
Content-Type: application/octet-stream;
	name="win32_thread.diff"
Content-Disposition: attachment;
	filename="win32_thread.diff"

\APR_1_0_0_RC4\apr\threadproc\win32>diff -u thread.c thread.c.old

--- thread.c    Fri Jul 16 17:13:34 2004
+++ thread.c.old        Fri Jul 16 17:19:47 2004
@@ -22,8 +22,7 @@
 #if APR_HAVE_PROCESS_H
 #include <process.h>
 #endif
-#include "apr_arch_misc.h"
-#include "apr_atomic.h"
+#include "apr_arch_misc.h"

 /* Chosen for us by apr_initialize */
 DWORD tls_apr_thread = 0;
@@ -79,22 +78,6 @@
     return thd->func(thd, thd->data);
 }

-static int apr_thread_close_os_handle (apr_thread_t *thd);
-static int apr_thread_close_os_handle (apr_thread_t *thd)
-{
-    if (!thd->td) {
-       return APR_SUCCESS;
-    }
-
-    if (!apr_atomic_read32(&thd->joined) && CloseHandle(thd->td)) {
-        thd->td = NULL;
-        return APR_SUCCESS;
-    }
-    else {
-        return apr_get_os_error();
-    }
-}
-
 APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
                                             apr_threadattr_t *attr,
                                             apr_thread_start_t func,
@@ -112,10 +95,6 @@
     (*new)->pool = pool;
     (*new)->data = data;
     (*new)->func = func;
-    (*new)->td  = NULL;
-    (*new)->exitval = APR_SUCCESS;
-    (*new)->exit_called=0;
-    (*new)->joined = 0;

     stat = apr_pool_create(&(*new)->pool, pool);
     if (stat != APR_SUCCESS) {
@@ -141,7 +120,8 @@
     }
 #endif
     if (attr && attr->detach) {
-       apr_thread_close_os_handle(*new);
+        CloseHandle((*new)->td);
+        (*new)->td = NULL;
     }

     return APR_SUCCESS;
@@ -151,10 +131,11 @@
                                           apr_status_t retval)
 {
     thd->exitval = retval;
-    thd->exit_called=1;
     apr_pool_destroy(thd->pool);
 #ifndef _WIN32_WCE
-    apr_thread_close_os_handle(thd);
+    if (thd->td) {
+        CloseHandle(thd->td);
+    }
     _endthreadex(0);
 #else
     ExitThread(0);
@@ -166,36 +147,25 @@
                                           apr_thread_t *thd)
 {
     apr_status_t rv;
-    DWORD hinfo;
-
-    apr_atomic_inc32(&thd->joined);
-    if(GetHandleInformation(thd->td,&hinfo))/*check for good handle*/
-    {
-       rv = WaitForSingleObject(thd->td, INFINITE);
-
-       if ( rv == WAIT_OBJECT_0 || rv == WAIT_ABANDONED ) {
-         *retval = thd->exitval;
-         apr_atomic_dec32(&thd->joined);
-         apr_thread_close_os_handle(thd);
-         return APR_SUCCESS;
-       }
-    }
-
-    /*assume thread finished ?*/
-    if(thd->exit_called)
-    {
-       *retval = thd->exitval;
-       return APR_SUCCESS;
-    }

-    apr_atomic_dec32(&thd->joined);
+    rv = WaitForSingleObject(thd->td, INFINITE);
+    if ( rv == WAIT_OBJECT_0 || rv == WAIT_ABANDONED) {
+        *retval = thd->exitval;
+        return APR_SUCCESS;
+    }
     /* Wait failed */
-    return apr_get_os_error();
+    return apr_get_os_error();;
 }

 APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
-{
-    return apr_thread_close_os_handle(thd);
+{
+    if (thd->td && CloseHandle(thd->td)) {
+        thd->td = NULL;
+        return APR_SUCCESS;
+    }
+    else {
+        return apr_get_os_error();
+    }
 }

 APR_DECLARE(void) apr_thread_yield()

------_=_NextPart_000_01C46B7D.785C5EA0--