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:[EMAIL PROTECTED] Sent:
Saturday, February 04, 2006 4:13 PM
To: Chris Storah
Cc: [email protected]
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);
