Bug 172586 - [Win] ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
Summary: [Win] ASSERTION FAILED: !HashTranslator::equal(KeyTraits::emptyValue(), key)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-05-25 02:35 PDT by Fujii Hironori
Modified: 2017-05-30 20:22 PDT (History)
12 users (show)

See Also:


Attachments
Patch (8.21 KB, patch)
2017-05-25 06:17 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (11.04 MB, application/zip)
2017-05-25 07:42 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fujii Hironori 2017-05-25 02:35:52 PDT
[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.
Comment 1 Fujii Hironori 2017-05-25 02:48:40 PDT
I'm not sure how to reproduce this bug.
I'm not sure Release build has this bug.
Comment 2 Fujii Hironori 2017-05-25 03:37:09 PDT
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?
Comment 3 Yusuke Suzuki 2017-05-25 05:21:08 PDT
(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.
Comment 4 Yusuke Suzuki 2017-05-25 06:17:47 PDT
Created attachment 311215 [details]
Patch
Comment 5 Build Bot 2017-05-25 07:42:19 PDT
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
Comment 6 Build Bot 2017-05-25 07:42:21 PDT
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
Comment 7 Yusuke Suzuki 2017-05-25 07:49:40 PDT
(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.
Comment 8 Brent Fulgham 2017-05-25 08:53:58 PDT
(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.
Comment 9 WebKit Commit Bot 2017-05-25 10:19:01 PDT
Comment on attachment 311215 [details]
Patch

Clearing flags on attachment: 311215

Committed r217432: <http://trac.webkit.org/changeset/217432>
Comment 10 WebKit Commit Bot 2017-05-25 10:19:03 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2017-05-30 20:22:41 PDT
<rdar://problem/32479745>