Re: [PATCH] WIN32 - fix apr_thread_join (PR: 28460) - v.2
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
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
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