On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov <span dir="ltr"><<a href="mailto:dimich@chromium.org">dimich@chromium.org</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Hi webkit-dev!<div><br></div><div>Trying to add ASSERTS to RefCounted objects (<a href="https://bugs.webkit.org/show_bug.cgi?id=31639" target="_blank">https://bugs.webkit.org/show_bug.cgi?id=31639</a>) I've added some more calls to WTF::currentThread() and started to get a lot of assertions here:</div>
<div><br></div><div><div><font face="'courier new', monospace">static ThreadIdentifier establishIdentifierForPthreadHandle(pthread_t& pthreadHandle)</font></div><div><font face="'courier new', monospace">{</font></div>
<div><font face="'courier new', monospace"> ASSERT(!identifierByPthreadHandle(pthreadHandle));</font></div><div><br></div><div>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. </div>
<div><br></div><div>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.</div>
<div><br></div><div>I'm thinking about changing the implementation to stop "auto-register" non-WTF threads.</div></div></blockquote><div><br></div><div>Wouldn't this break binary compatibility?</div><div>
<br></div><div>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?</div>
<div><br></div><div>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?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div><div>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.</div>
<div><br></div><div>The currentThread() would look like this:</div><div><br></div><div><div><font face="'courier new', monospace">ThreadIdentifier currentThread()</font></div><div><font face="'courier new', monospace">{</font></div>
<div><font face="'courier new', monospace"> pthread_t currentThread = pthread_self();</font></div><div><font face="'courier new', monospace"> if (ThreadIdentifier id = identifierByPthreadHandle(currentThread))</font></div>
<div><font face="'courier new', monospace"> return id;</font></div><div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace"> ASSERT_NOT_REACHED();</font></div>
<div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace"> // Either the thread is not created by WTF::CreateThread() or registered by WTF::registerThread(), or we are getting</font></div>
<div><font face="'courier new', monospace"> // a call from thread-specific destructor after WTF::detachThread() was called and ThreadIdentifier removed.</font></div><div><font face="'courier new', monospace"> // Neither scenario permits reliable thread id tracking, so we can not return a meaningful ThreadIdentifier here.</font></div>
<div><font face="'courier new', monospace"> // Normally ThreadIdentifiers are compared so lets generate a fake one-time ThreadIdentifier to fail comparison, if</font></div><div><font face="'courier new', monospace"> // it is in fact done.</font></div>
<div><font face="'courier new', monospace"> static ThreadIdentifier fakeId = minFakeIdentifierCount;</font></div><div><font face="'courier new', monospace"><br>
</font></div><div><font face="'courier new', monospace"> if (fakeId == maxFakeIdentifierCount)</font></div><div><font face="'courier new', monospace"> fakeId = minFakeIdentifierCount;</font></div>
<div><font face="'courier new', monospace"><br></font></div><div><font face="'courier new', monospace"> return fakeId++;</font></div><div><font face="'courier new', monospace">}</font></div>
<div><br></div><div>What would you say? Any bad feelings about that?</div><div><br></div><font color="#888888"><div>Dmitry</div></font></div></div>
<br>_______________________________________________<br>
webkit-dev mailing list<br>
<a href="mailto:webkit-dev@lists.webkit.org">webkit-dev@lists.webkit.org</a><br>
<a href="http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev" target="_blank">http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev</a><br>
<br></blockquote></div><br>