Bug 133553 – [Win] Debug builds of TestWebKitAPI are crashing

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Brent Fulgham]

The build bots are falling (in debug builds) to run the "run-api-tests" due to a crash.

Build log:

    perl ./Tools/Scripts/run-api-tests --debug --verbose --no-build
     in dir /home/buildbot/slave/win-debug-tests/build (timeout 1200 secs)
     using PTY: False
    Error (2): The system cannot find the file specified.

    cygpath: can't convert empty path
    Failed to build list of tests!
    program finished with exit code 3

The cygpath warning is not a problem. The actual crash is as follows:

 	KernelBase.dll!_RaiseException@16()	Unknown
 	[External Code]	
 	msvcr120.dll!Concurrency::details::SchedulerBase::SchedulerBase(const Concurrency::SchedulerPolicy & policy) Line 159	C++
 	msvcr120.dll!Concurrency::details::SchedulerBase::CreateWithoutInitializing(const Concurrency::SchedulerPolicy & policy) Line 285	C++
 	msvcr120.dll!Concurrency::details::SchedulerBase::GetDefaultScheduler() Line 657	C++
 	msvcr120.dll!Concurrency::details::SchedulerBase::CreateContextFromDefaultScheduler() Line 571	C++
 	msvcr120.dll!Concurrency::details::SchedulerBase::CurrentContext()	C++
 	msvcr120.dll!Concurrency::details::LockQueueNode::LockQueueNode(unsigned int timeout) Line 620	C++
 	msvcr120.dll!Concurrency::critical_section::lock() Line 1031	C++
 	msvcp120.dll!mtx_do_lock(_Mtx_internal_imp_t * * mtx, const xtime * target) Line 67	C++
 	msvcp120.dll!_Mtx_lock(_Mtx_internal_imp_t * * mtx) Line 153	C++
 	WebKit.dll!std::_Mtx_lockX(_Mtx_internal_imp_t * * _Mtx) Line 68	C++
 	WebKit.dll!std::_Mutex_base::lock() Line 41	C++
 	WebKit.dll!std::lock_guard<std::mutex>::lock_guard<std::mutex>(std::mutex & _Mtx) Line 184	C++
>	WebKit.dll!WTF::HashTable<WebCore::Widget const *,WTF::KeyValuePair<WebCore::Widget const *,WebCore::RenderWidget *>,WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::Widget const *,WebCore::RenderWidget *> >,WTF::PtrHash<WebCore::Widget const *>,WTF::HashMap<WebCore::Widget const *,WebCore::RenderWidget *,WTF::PtrHash<WebCore::Widget const *>,WTF::HashTraits<WebCore::Widget const *>,WTF::HashTraits<WebCore::RenderWidget *> >::KeyValuePairTraits,WTF::HashTraits<WebCore::Widget const *> >::invalidateIterators() Line 1230	C++
 	WebKit.dll!WTF::HashTable<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *,WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *,WTF::IdentityExtractor,WTF::ListHashSetNodeHashFunctions<WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions>,WTF::HashTraits<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *>,WTF::HashTraits<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *> >::~HashTable<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *,WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *,WTF::IdentityExtractor,WTF::ListHashSetNodeHashFunctions<WebCore::DOMNamedFlowCollection::DOMNamedFlowHashFunctions>,WTF::HashTraits<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *>,WTF::HashTraits<WTF::ListHashSetNode<WTF::RefPtr<WebCore::WebKitNamedFlow>,1> *> >() Line 352	C++
 	[External Code]	

This seems to have been going for at least r169288.

[Brent Fulgham]

Run output:

bfulgham@fulgbr-pc ~/Projects/WebKit
$ run-api-tests --debug --no-build -v
RUnning /cygdrive/c/Projects/WebKit/OpenSource/WebKitBuild/Debug/bin32/TestWebKi
tAPI.exe
WebCoreLayoutUnit.
  LayoutUnitInt
  LayoutUnitFloat
  LayoutUnitRounding
  LayoutUnitSnapSizeToPixel
  LayoutUnitMultiplication
  LayoutUnitDivision
  LayoutUnitCeil
  LayoutUnitFloor
WebViewDestruction.
  NoInitWithFrame
  CloseWithoutInitWithFrame
WebViewDestructionWithHostWindow.
  CloseWithoutDestroyViewWindow
  DestroyViewWindowWithoutClose
  CloseThenDestroyViewWindow
  DestroyViewWindowThenClose
  DestroyHostWindow
  DestroyHostWindowThenClose
  CloseThenDestroyHostWindow
RetainPtr.
  AdoptCF
RetainPtrHashing.
  HashSet
  HashMapKey
  HashMapValue
WTF.
  Checked_int8_t
  Checked_int16_t
  Checked_int32_t
  Checked_uint32_t
  Checked_int64_t
  Checked_uint64_t
  Lrint
  clampToIntLong
  clampToIntLongLong
  clampToIntegerFloat
  clampToIntegerDouble
  clampToFloat
  clampToUnsignedLong
  clampToUnsignedLongLong
  MediaTime
  SaturatedArithmeticAddition
  SaturatedArithmeticSubtraction
  StringHasher
  StringHasher_addCharacter
  StringHasher_addCharacters
  StringHasher_addCharactersAssumingAligned
  StringHasher_computeHash
  StringHasher_computeHashAndMaskTop8Bits
  StringHasher_hashMemory
  StringOperators
FunctionalTest.
  Basic
  UnaryBind
  BinaryBind
  MemberFunctionBind
  MemberFunctionBindRefDeref
  RefCountedStorage
WTF_HashMap.
  HashTableIteratorComparison
  DoubleHashCollisions
  MoveOnlyValues
  MoveOnlyKeys
  InitializerList
WTF_MD5.
  Computation
WTF_Ref.
  Basic
  Assignment
  ReturnValue
WTF_RefPtr.
  Basic
  AssignPassRefToRefPtr
  Adopt
  Assignment
  Swap
  ReleaseNonNull
  Release
  ReturnValue
WTF_SHA1.
  Computation
WTF_Vector.
  Basic
  Iterator
  OverloadedOperatorAmpersand
  AppendLast
  InitializerList
  Reverse
  ReverseIterator
  MoveOnly_UncheckedAppend
  MoveOnly_Append
  MoveOnly_Insert
  MoveOnly_TakeLast
  VectorOfVectorsOfVectorsInlineCapacitySwap
Result = 32512
Failed to build list of tests!

bfulgham@fulgbr-pc ~/Projects/WebKit
$

[Brent Fulgham]

*** Bug 137028 has been marked as a duplicate of this bug. ***

[Brent Fulgham]

This is failing when the destructor for a global static HashTable is called. Specifically, the Windows Concurrency runtime is not happy with the state of the mutex we use for the iterator checks in debug builds.

[Alexey Proskuryakov]

> global static HashTable is called

The fix is probably to make the variable a NeverDestroyed - we normally don't allow any code that runs at library load or unload time in WebKit.

[Brent Fulgham]

(In reply to comment #4)
> > global static HashTable is called
> 
> The fix is probably to make the variable a NeverDestroyed - we normally don't allow any code that runs at library load or unload time in WebKit.

Great! Let me give that a try and see if it resolves the problem.

[Brent Fulgham]

The bug seems to be caused by the following line in Node.cpp:

DEFINE_DEBUG_ONLY_GLOBAL(HashSet<Node*>, ignoreSet, );

The static destructor on this seems to get called after the Concurrency system on Windows has already torn down the mutex, resulting in the error.

[Brent Fulgham]

Actually, it looks like there are a handful of similar things:

Node.cpp:: ignoreSet
WebView.cpp: pendingDeleteBackingStoreSet
WebPreferences.cpp: webPreferencesInstances
WebKitDLL.cpp: gClassNameCount

I'm going to try to NeverDestroyed<> them and see if that resolves the issue.

[Brent Fulgham]

Created attachment 238676 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 238676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238676&action=review

> Source/WebKit/win/WebKitDLL.cpp:58
> +HashCountedSet<String>* gClassNameCount()

Why not return a reference here?

[Brent Fulgham]

Comment on attachment 238676 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238676&action=review

>> Source/WebKit/win/WebKitDLL.cpp:58
>> +HashCountedSet<String>* gClassNameCount()
> 
> Why not return a reference here?

The original code was marked "extern "C"", so I can't use a C++ reference.

[Alexey Proskuryakov]

Does using a reference actually break the build? I'd be very surprised if references were forbidden, but "HashCountedSet<String>" was OK.

[Brent Fulgham]

You are right. The reference works fine. I'll make that change while landing

[Brent Fulgham]

Committed r173988: <http://trac.webkit.org/changeset/173988>