Re: [PATCH] WIN32 - fix apr_thread_join (PR: 28460) - v.2

2004-08-28 Thread William A. Rowe, Jr.
At 02:24 PM 8/27/2004, Mladen Turk wrote:
Cliff Woolley wrote:

On Fri, 27 Aug 2004, Mladen Turk wrote:

c) If thread_exit was never called before thread_join
   do not set the retval but rather return APR_INCOMPLETE.

That's really what the unix impl does?

Probably not :)
Didn't test what happens if you call the thread_join
without ever calling thread_exit.

 APR_INCOMPLETE was supposed to
mean that partial results were returned but just not quite everything the
caller asked for (as in the case of stat()ing a file but only getting back
some of the information the user wanted rather than all).  Surely this is
an error condition, right?  In that case, one of the APR_E* codes would be
more appropriate.  Or maybe I'm not correctly understanding the situation
you're describing...

APR_INCOMLETE says :
The operation was incomplete although some processing
was performed and the results are partially valid

This rv can only happen if the code has been wrongly written,
meaning that the user didn't call the thread_exit inside the
thread func, but sill wishes to retrieve the thread exit
value using thread_join.

Because the thread handle is closed the operation is
incomplete, meaning: OK, I can close the thread, but
can not give you the thread's return value, cause you
never set that.

Anyhow, any error value will do as long as it makes sense :)

It doesn't because you didn't get a partial-results, you got no
results (it wasn't done.)  I like the idea of researching what
Unix reports and matching it.

Bill




[PATCH] WIN32 - fix apr_thread_join (PR: 28460) - v.2

2004-08-27 Thread Mladen Turk
Hi,
Second try :)
To be consistent, the patch now follows the unix behavior.
a) If the thread is detached don't allow thread_join
   but rather return APR_DETACHED.
b) If not detached, expect that thread_join will be
   called.
c) If thread_exit was never called before thread_join
   do not set the retval but rather return APR_INCOMPLETE.

cvs diff -u thread.c (in directory 
C:\WRKTOOLS\WINCVS\DATA\apache\apr\threadproc\win32\)
Index: thread.c
===
RCS file: /home/cvspublic/apr/threadproc/win32/thread.c,v
retrieving revision 1.57
diff -u -r1.57 thread.c
--- thread.c	10 Jun 2004 10:57:25 -	1.57
+++ thread.c	27 Aug 2004 18:34:44 -
@@ -85,6 +85,7 @@
 {
 apr_status_t stat;
 	unsigned temp;
+HANDLE handle;

 (*new) = (apr_thread_t *)apr_palloc(pool, sizeof(apr_thread_t));
@@ -95,7 +96,7 @@
 (*new)-pool = pool;
 (*new)-data = data;
 (*new)-func = func;
-
+(*new)-td   = NULL;
 stat = apr_pool_create((*new)-pool, pool);
 if (stat != APR_SUCCESS) {
 return stat;
@@ -105,14 +106,14 @@
  * same size as the calling thread.
  */
 #ifndef _WIN32_WCE
-if (((*new)-td = (HANDLE)_beginthreadex(NULL,
+if ((handle = (HANDLE)_beginthreadex(NULL,
 attr  attr-stacksize  0 ? attr-stacksize : 0,
 (unsigned int (APR_THREAD_FUNC *)(void 
*))dummy_worker,
 (*new), 0, temp)) == 0) {
 return APR_FROM_OS_ERROR(_doserrno);
 }
 #else
-   if (((*new)-td = CreateThread(NULL,
+   if ((handle = CreateThread(NULL,
 attr  attr-stacksize  0 ? attr-stacksize : 0,
 (unsigned int (APR_THREAD_FUNC *)(void 
*))dummy_worker,
 (*new), 0, temp)) == 0) {
@@ -120,9 +121,10 @@
 }
 #endif
 if (attr  attr-detach) {
-CloseHandle((*new)-td);
-(*new)-td = NULL;
+CloseHandle(handle);
 }
+else
+(*new)-td = handle;

 return APR_SUCCESS;
 }
@@ -132,10 +134,8 @@
 {
 thd-exitval = retval;
 apr_pool_destroy(thd-pool);
+thd-pool = NULL;
 #ifndef _WIN32_WCE
-if (thd-td) {
-CloseHandle(thd-td);
-}
 _endthreadex(0);
 #else
 ExitThread(0);
@@ -146,15 +146,26 @@
 APR_DECLARE(apr_status_t) apr_thread_join(apr_status_t *retval,
   apr_thread_t *thd)
 {
-apr_status_t rv;
-
+apr_status_t rv = APR_SUCCESS;
+
+if (!thd-td) {
+/* Can not join on detached threads */
+return APR_DETACH;
+}
 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();;
+/* If the thread_exit has been called */
+if (!thd-pool)
+*retval = thd-exitval;
+else
+rv = APR_INCOMPLETE;
+}
+else
+rv = apr_get_os_error();
+CloseHandle(thd-td);
+thd-td = NULL;
+
+return rv;
 }
 APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)


Regards,
MT.


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH] WIN32 - fix apr_thread_join (PR: 28460) - v.2

2004-08-27 Thread Mladen Turk
Cliff Woolley wrote:
On Fri, 27 Aug 2004, Mladen Turk wrote:

c) If thread_exit was never called before thread_join
   do not set the retval but rather return APR_INCOMPLETE.

That's really what the unix impl does?
Probably not :)
Didn't test what happens if you call the thread_join
without ever calling thread_exit.
 APR_INCOMPLETE was supposed to
mean that partial results were returned but just not quite everything the
caller asked for (as in the case of stat()ing a file but only getting back
some of the information the user wanted rather than all).  Surely this is
an error condition, right?  In that case, one of the APR_E* codes would be
more appropriate.  Or maybe I'm not correctly understanding the situation
you're describing...
APR_INCOMLETE says :
The operation was incomplete although some processing
was performed and the results are partially valid
This rv can only happen if the code has been wrongly written,
meaning that the user didn't call the thread_exit inside the
thread func, but sill wishes to retrieve the thread exit
value using thread_join.
Because the thread handle is closed the operation is
incomplete, meaning: OK, I can close the thread, but
can not give you the thread's return value, cause you
never set that.
Anyhow, any error value will do as long as it makes sense :)
Regards,
MT.


smime.p7s
Description: S/MIME Cryptographic Signature