Bug 172586 – [Win] ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Fujii Hironori]

[WinCairo] ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)

WinCairo port MiniBrowser, Debug build, trunk@217407
I often see a following assertion failure.

> ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
> C:\webkit\ga\WebKitBuild\Debug\DerivedSources\ForwardingHeaders\wtf/HashTable.h(587) : WTF::HashTable<unsigned int,struct WTF::KeyValuePair<unsigned int,class WTF::ThreadHolder *>,struct WTF::KeyValuePairKeyExtractor<struct WTF::KeyValuePair<unsigned int,class WTF::ThreadHolder *> >,struct WTF::IntHash<unsigned int>,struct WTF::HashMap<unsigned int,class WTF::ThreadHolder *,struct WTF::IntHash<unsigned int>,struct WTF::HashTraits<unsigned int>,struct WTF::HashTraits<class WTF::ThreadHolder *> >::KeyValuePairTraits,struct WTF::HashTraits<unsigned int> >::checkKey

Callstack:

> WTF.dll!WTFCrash() Line 292	C++
> WTF.dll!WTF::HashTable<unsigned int,WTF::KeyValuePair<unsigned int,WTF::ThreadHolder * __ptr64>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned int,WTF::ThreadHolder * __ptr64> >,WTF::IntHash<unsigned int>,WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::KeyValuePairTraits,WTF::HashTraits<unsigned int> >::checkKey<WTF::HashMapTranslator<WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::KeyValuePairTraits,WTF::IntHash<unsigned int> >,unsigned int>(const unsigned int & key) Line 587	C++
> WTF.dll!WTF::HashTable<unsigned int,WTF::KeyValuePair<unsigned int,WTF::ThreadHolder * __ptr64>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned int,WTF::ThreadHolder * __ptr64> >,WTF::IntHash<unsigned int>,WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::KeyValuePairTraits,WTF::HashTraits<unsigned int> >::add<WTF::HashMapTranslator<WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::KeyValuePairTraits,WTF::IntHash<unsigned int> >,unsigned int,WTF::ThreadHolder * __ptr64 & __ptr64>(unsigned int && key, WTF::ThreadHolder * & extra) Line 866	C++
> WTF.dll!WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::inlineAdd<unsigned int,WTF::ThreadHolder * __ptr64 & __ptr64>(unsigned int && key, WTF::ThreadHolder * & value) Line 331	C++
> WTF.dll!WTF::HashMap<unsigned int,WTF::ThreadHolder * __ptr64,WTF::IntHash<unsigned int>,WTF::HashTraits<unsigned int>,WTF::HashTraits<WTF::ThreadHolder * __ptr64> >::add<WTF::ThreadHolder * __ptr64 & __ptr64>(unsigned int && key, WTF::ThreadHolder * & mapped) Line 373	C++
> WTF.dll!WTF::ThreadHolder::platformInitialize(WTF::ThreadHolder * holder) Line 86	C++
> WTF.dll!WTF::ThreadHolder::initialize(WTF::Thread & thread) Line 62	C++
> WTF.dll!WTF::wtfThreadEntryPoint(void * param) Line 159	C++
> WTF.dll!thread_start<unsigned int (__cdecl*)(void * __ptr64)>(void * const parameter) Line 115	C++
> [External Code]	

I checked 'key' argument of inlineAdd() by using VS debugger, it was 0.
This is the code of ThreadHolder::platformInitialize:

> void ThreadHolder::platformInitialize(ThreadHolder* holder)
> {
>     std::unique_lock<std::mutex> locker(threadMapMutex());
>     threadMap().add(holder->thread().id(), holder);
> }

holder->thread().id() seemed to return 0.
But, WTF::Thread::m_id was 12824.
This is weird.

[Fujii Hironori]

I'm not sure how to reproduce this bug.
I'm not sure Release build has this bug.

[Fujii Hironori]

This is a data race of WTF::Thread::m_id.
m_id is set at Thread::establish() in the parent thread.
But, this is too late.

It's easy to reproduce this bug by applying the following patch.

> diff --git a/Source/WTF/wtf/ThreadingWin.cpp b/Source/WTF/wtf/ThreadingWin.cpp
> index 727044f87bd..d9525552c0f 100644
> --- a/Source/WTF/wtf/ThreadingWin.cpp
> +++ b/Source/WTF/wtf/ThreadingWin.cpp
> @@ -177,7 +177,7 @@ RefPtr<Thread> Thread::createInternal(ThreadFunction entryPoint, void* data, con
>  #endif
>          return 0;
>      }
> -
> +    Sleep(3000);
>      // The thread will take ownership of invocation.
>      ThreadFunctionInvocation* leakedInvocation = invocation.release();
>      UNUSED_PARAM(leakedInvocation);
>

Could you take a look, Yusuke?

[Yusuke Suzuki]

(In reply to Fujii Hironori from comment #2)
> This is a data race of WTF::Thread::m_id.
> m_id is set at Thread::establish() in the parent thread.
> But, this is too late.
> 
> It's easy to reproduce this bug by applying the following patch.
> 
> > diff --git a/Source/WTF/wtf/ThreadingWin.cpp b/Source/WTF/wtf/ThreadingWin.cpp
> > index 727044f87bd..d9525552c0f 100644
> > --- a/Source/WTF/wtf/ThreadingWin.cpp
> > +++ b/Source/WTF/wtf/ThreadingWin.cpp
> > @@ -177,7 +177,7 @@ RefPtr<Thread> Thread::createInternal(ThreadFunction entryPoint, void* data, con
> >  #endif
> >          return 0;
> >      }
> > -
> > +    Sleep(3000);
> >      // The thread will take ownership of invocation.
> >      ThreadFunctionInvocation* leakedInvocation = invocation.release();
> >      UNUSED_PARAM(leakedInvocation);
> >
> 
> Could you take a look, Yusuke?

Right. This bug is introduced in https://bugs.webkit.org/show_bug.cgi?id=171029.
Thread itself's id exposure to users is ensured by using context->creationMutex. But ThreadHolder touches it before it is set.

[Yusuke Suzuki]

Created attachment 311215 [details]
Patch

[Build Bot]

Comment on attachment 311215 [details]
Patch

Attachment 311215 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3813015

New failing tests:
webrtc/peer-connection-audio-mute.html
fast/css/target-fragment-match.html

[Build Bot]

Created attachment 311216 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

[Yusuke Suzuki]

(In reply to Build Bot from comment #6)
> Created attachment 311216 [details]
> Archive of layout-test-results from ews121 for ios-simulator-wk2
> 
> The attached test failures were seen while running run-webkit-tests on the
> ios-sim-ews.
> Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5

I don't think it is related b/c this patch does not change code in iOS.

[Brent Fulgham]

(In reply to Yusuke Suzuki from comment #7)
> (In reply to Build Bot from comment #6)
> > Created attachment 311216 [details]
> > Archive of layout-test-results from ews121 for ios-simulator-wk2
> > 
> > The attached test failures were seen while running run-webkit-tests on the
> > ios-sim-ews.
> > Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
> 
> I don't think it is related b/c this patch does not change code in iOS.

The WebRTC failure is clearly not related -- this is failing without your patch.

I think the iOS EWS is in bad shape at the moment.

[WebKit Commit Bot]

Comment on attachment 311215 [details]
Patch

Clearing flags on attachment: 311215

Committed r217432: <http://trac.webkit.org/changeset/217432>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/32479745>