On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov <[email protected]> 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 > [email protected] > http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev > >
_______________________________________________ webkit-dev mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
