Need to change WTF::isMainThread() for pthreads to work correctly for thread-specific destructors

• Component: Web Template Framework
• Status: RESOLVED DUPLICATE

[Dmitry Titov]

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.

[Dmitry Titov]

Created attachment 29409 [details]
Proposed patch

[Darin Adler]

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]

(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 on attachment 29409 [details]
Proposed patch

removing the r? since this needs more work.

[Dmitry Titov]

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 ***