Bug 25138

Summary: Need to change WTF::isMainThread() for pthreads to work correctly for thread-specific destructors
Product: WebKit Reporter: Dmitry Titov <dimich>
Component: Web Template FrameworkAssignee: Dmitry Titov <dimich>
Status: RESOLVED DUPLICATE    
Severity: Normal CC: ap, aroben
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Attachments:
Description Flags
Proposed patch none

Dmitry Titov
Reported 2009-04-10 16:32:51 PDT
Pthreads run destructors of thread-specific data after thread exits from its thread proc. On non-DARWIN or CHROMIUM flavors of pthreads-based ports, this causes the ThreadIdentifier to be removed from the WTF ThreadMap before those destructors are run. Because of this, the check in isMainThread(): return currentThread() == mainThreadIdentifier; will insert the new ThreadIdentifier for the exiting thread which will not be removed from the map and cause asserts later in establishIdentifierForPthreadHandle when a new thread with the same thread ID will be created (pthreads reuse their ids). Fixed with replacing the check with return pthread_equal(pthread_self(), mainThreadPthreadID); Since other platform use other methods of dealing with TLS, this is pthread-only fix.
Attachments
Proposed patch (1.93 KB, patch)
2009-04-10 16:35 PDT, Dmitry Titov 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Dmitry Titov
Comment 1 2009-04-10 16:35:57 PDT
Created attachment 29409 [details] Proposed patch
Darin Adler
Comment 2 2009-04-10 16:51:51 PDT
Comment on attachment 29409 [details] Proposed patch Seems OK to fix isMainThread this way assuming we can live with not being able to fix currentThread(). But what about use of the currentThread() function in thread-specific destructors? Is there a way we can fix that?
Dmitry Titov
Comment 3 2009-04-10 17:30:03 PDT
(In reply to comment #2) > (From update of attachment 29409 [details] [review]) > Seems OK to fix isMainThread this way assuming we can live with not being able > to fix currentThread(). > > But what about use of the currentThread() function in thread-specific > destructors? Is there a way we can fix that? > I'm not sure how to fix currentThread(), especially considering the order of thread-specific destructors is unspecified and pthread_t ids can be reused right after the previous thread using it is finally terminated. Perhaps it's better to say the ThreadIdentifier is undefined outside of the create/detach lifespan. Since we don't use currentThread() now in the thread-specific destructors, I think it would be useful to have a currentThread() return -1 and ASSERT if called outside of create/detach thread lifetime or if it is not the thread that's called initializeThreading(). This way, the threads from embedder created outside of WTF would not get assigned WTF ThreadIdentifier, nor create a problem with cleaning those out of the ThreadMap (since they wouldn't use WTF::detachThread). Also, it would actively discourage using currentThread() in thread-specific destructors in the future. If there will be a need to use WTF threading functions on threads created outside of WTF, we can always add something like WTF::initializeThreadingForCurrentThread with requirement to call detachThread later to reclaim WTF. Do you find this a reasonable direction?
Dmitry Titov
Comment 4 2009-04-13 11:21:34 PDT
Comment on attachment 29409 [details] Proposed patch removing the r? since this needs more work.
Dmitry Titov
Comment 5 2009-04-23 14:14:36 PDT
Resolving this one as a duplicate of 25348, since that one solves the same issue in a more complete way. *** This bug has been marked as a duplicate of 25348 ***
Note You need to log in before you can comment on or make changes to this bug.