Re: [webkit-dev] WTF::currentThread() and non-WTF threads
On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov dim...@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@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] WTF::currentThread() and non-WTF threads
Thanks for sharing your thoughts!
On Fri, Dec 11, 2009 at 2:44 AM, Jeremy Orlow jor...@chromium.org wrote:
On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov dim...@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?
If you mean nightly builds, OSX Safari runs fine with this. On Windows it's
going to be ok because we use OS-supplied ThreadIdentifiers there.
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?
I think I see what you mean. I tried to figure out if it is possible. There
is a callback invoked on destruction of a thread-specific slot... Perhaps it
can be used to un-register the thread if we add a thread-specific slot to
all incoming threads. The problem with those is that they are called in
unspecified order so another thread-specific destructor could re-register a
thread again, by calling currentThread(). Perhaps there is a way to do this
cleanly, it escapes me at the moment...
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?
Most usages of currentThread() are like this:
ASSERT(m_myThreadId == currentThread());
Basically, they verify that the object is called on the same thread as
created... Returning same identifier for different threads satisfies this
check even if the treads are different. If currentThread() returned the
different identifier each time in case of un-registered thread, it would
force the developer to either use WTF::createThread or WTF::registerThread.
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@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] WTF::currentThread() and non-WTF threads
On Fri, Dec 11, 2009 at 4:08 PM, Dmitry Titov dim...@chromium.org wrote:
Thanks for sharing your thoughts!
On Fri, Dec 11, 2009 at 2:44 AM, Jeremy Orlow jor...@chromium.org wrote:
On Thu, Dec 10, 2009 at 6:48 PM, Dmitry Titov dim...@chromium.orgwrote:
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?
If you mean nightly builds, OSX Safari runs fine with this. On Windows it's
going to be ok because we use OS-supplied ThreadIdentifiers there.
WebKit is a framework on macso I was wondering if any app that depends
on the framework will work OK. I have no idea, but my guess is this would
be an issue.
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?
I think I see what you mean. I tried to figure out if it is possible. There
is a callback invoked on destruction of a thread-specific slot... Perhaps it
can be used to un-register the thread if we add a thread-specific slot to
all incoming threads. The problem with those is that they are called in
unspecified order so another thread-specific destructor could re-register a
thread again, by calling currentThread(). Perhaps there is a way to do this
cleanly, it escapes me at the moment...
What if the thread local storage kept track of the threads identifier? If
it's empty, then we need to auto-register. If it's there, then we use that.
I think that would avoid the race you mentioned.
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?
Most usages of currentThread() are like this:
ASSERT(m_myThreadId == currentThread());
Basically, they verify that the object is called on the same thread as
created... Returning same identifier for different threads satisfies this
check even if the treads are different. If currentThread() returned the
different identifier each time in case of un-registered thread, it would
force the developer to either use WTF::createThread or WTF::registerThread.
If all non-WTF threads used the same identifier, then all blah ==
currentThread() asserts would work. Some of the less common blah !=
currentThread() asserts could break if the current thread and the one it's
comparing to are both non-WTF. We could probably find ways around all of
those asserts.
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
[webkit-dev] WTF::currentThread() and non-WTF threads
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. 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@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
