apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chris Storah <chris.sto...@orionsmg.com>
Subject Re: [PATCH] APR thread handle leak on Windows
Date Wed, 08 Feb 2006 00:31:03 GMT
Yes, I can see the conflict with join/detach.

I use the apr_thread_join in most of my code, but this problem appeared
in old Windows code that had been ported to use APR and did not have a
requirement to check the status so I missed the join. The non-patch  
version
now works without leaks after putting the join into my code.

Agreed, extra doxygen comments would be handy - especially from any
Windows only programmers that have not used thread joining before,  
and as
a reminder to people who forget to put it in :)

Thanks,
Chris


> Thanks Ed, I knew this sounded familiar.  At that time I'm unsure  
> we even
> handled the 'unix case' right, but now they are semantically  
> identical.
>
> I'd veto any change to make a platform 'do the right thing' by  
> abusing the
> calling sematics.  You must use the API portably.
>
> On that note, perhaps apr_thread_create() needs some additional  
> doxygen
> comments about how-to-close a thread?
>
> Bill
>
> Ed Holyat wrote:
>> This same discussion happened about a year ago.  The patch is not  
>> thread safe, because of a race condition in closing the
>> handle in the thread and joining the thread and/or detaching from
>> another thread.  I believe that the thread tests use to crash on  
>> win32.
>> The answer was to enforce the UNIX style that someone quoted from the
>> man pages: basically saying that apr_thread_join must be called.
>> -----Original Message-----
>> From: William A. Rowe, Jr. [mailto:wrowe@rowe-clan.net] Sent:  
>> Saturday, February 04, 2006 4:13 PM
>> To: Chris Storah
>> Cc: dev@apr.apache.org
>> Subject: Re: [PATCH] APR thread handle leak on Windows
>> Chris, and devs, I'm confused;
>> the thd->td handle *is* closed in apr_thread_join.  If we deploy this
>> patch
>> to accomodate a particular programming style, we lose valuable return
>> context
>> information, but more importantly, if you aren't invoking
>> apr_thread_join, then
>> on many flavors of *nix you aren't farming the zombie threads.
>> Note we also properly close thd->td within apr_thread_detach, for  
>> much
>> the
>> same reasons one or the other must be invoked on *nix.
>> This patch is problematic for me; anyone else have some thoughts to
>> share?
>> Bill
>> Chris Storah wrote:
>>> Enclosed is a patch to fix a leak in APR threads, due to  
>>> _endthreadex
>>> not automatically closing the handle (unlike _endthread).
>>>
>>> Chris
>>>
>>>
>>>
>> --------------------------------------------------------------------- 
>> ---
>>> --- thread.c	2005-12-30 15:44:05.000000000 +1100
>>> +++ thread.c.orig	2005-12-29 10:05:45.000000000 +1100
>>> @@ -137,10 +137,6 @@
>>>     apr_pool_destroy(thd->pool);
>>>     thd->pool = NULL;
>>> #ifndef _WIN32_WCE
>>> -	/* need to close the handle as _endthreadex does not
>> automatically
>>> -	 * do this - unlike _endthread
>>> -	 */
>>> -	CloseHandle(thd->td);
>>>     _endthreadex(0);
>>> #else
>>>     ExitThread(0);
>


Mime
View raw message