[webkit-dev] WTF::currentThread() and non-WTF threads

[Jeremy Orlow]

On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov <dimich at chromium.org> wrote:

> Hi webkit-dev!
>
> Trying to add ASSERTS to RefCounted objects (
> https://bugs.webkit.org/show_bug.cgi?id=31639) I've added some more calls
> to WTF::currentThread() and started to get a lot of assertions here:
>
> static ThreadIdentifier establishIdentifierForPthreadHandle(pthread_t&
> pthreadHandle)
> {
>     ASSERT(!identifierByPthreadHandle(pthreadHandle));
>
> Here is what happens: A thread that is not created by WTF::CreateThread is
> calling currentThread() in hope to get a ThreadIdentifier. It doesn't have
> it so we create it and put into ThreadMap. Normally, WTF threads call
> WTF::detachThread() that removes the ThreadIdentifier from the map. The
> 'other' threads do not do so - so the TheradIdentifier remains in the map.
>
> On OSX, pthreads reuse system handles when one thread terminates and
> another starts... That easily leads to threads that run sequentially to have
> the same handle, and the same ThreadIdentifier - since it is still in the
> threadMap, which makes currentThread() kind of useless since it returns the
> same id for different threads, the very thing the threadMap is tryign to
> avoid.
>
> I'm thinking about changing the implementation to stop "auto-register"
> non-WTF threads.
>

Wouldn't this break binary compatibility?

Does pthreads have any facility to register a callback that's called on
thread shutdown?  Could we use some sort of thread local storage to keep
track of whether that thread has been auto-registered yet?

Maybe all non-registered threads could just use a default thread identifier
and some sort of warning message could be logged the first time this
happens?


> Instead, lets add a new function, WTF::registerThread() that would
> establish the ThreadIdentifier for a thread not created by WTF::createThread
> - and assert in currentThread() if the current thread is unknown to WTF.
> This way, we could find all the cases of such threads and equip them with
> registerThread()/detachThread() pair that will keep the thread map in a good
> state.
>
> The currentThread() would look like this:
>
> ThreadIdentifier currentThread()
> {
>     pthread_t currentThread = pthread_self();
>     if (ThreadIdentifier id = identifierByPthreadHandle(currentThread))
>         return id;
>
>     ASSERT_NOT_REACHED();
>
>     // Either the thread is not created by WTF::CreateThread() or
> registered by WTF::registerThread(), or we are getting
>     // a call from thread-specific destructor after WTF::detachThread() was
> called and ThreadIdentifier removed.
>     // Neither scenario permits reliable thread id tracking, so we can not
> return a meaningful ThreadIdentifier here.
>     // Normally ThreadIdentifiers are compared so lets generate a fake
> one-time ThreadIdentifier to fail comparison, if
>     // it is in fact done.
>     static ThreadIdentifier fakeId = minFakeIdentifierCount;
>
>     if (fakeId == maxFakeIdentifierCount)
>         fakeId = minFakeIdentifierCount;
>
>     return fakeId++;
> }
>
> What would you say? Any bad feelings about that?
>
> Dmitry
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091211/779df95a/attachment.html>