Bug 180602 – Crash in WTF::Atomic<unsigned char>::compareExchangeWeak + 36

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

The following crash was seen on El Capitan Debug WK2 with LayoutTests compositing/video/video-clip-change-src.html and credentials/idlharness.html

https://build.webkit.org/results/Apple%20El%20Capitan%20Debug%20WK2%20(Tests)/r225684%20(4532)/results.html

Thread 1 Crashed:: Dispatch queue: WebResourceLoadStatisticsStore Process Data Queue
0   com.apple.WebKit              	0x000000011396eb2f WTF::Atomic<unsigned char>::compareExchangeWeak(unsigned char, unsigned char, std::__1::memory_order) + 367 (Atomics.h:87)
1   com.apple.WebKit              	0x000000011396e9b3 WTF::LockAlgorithm<unsigned char, (unsigned char)1, (unsigned char)2, WTF::EmptyLockHooks<unsigned char> >::lockFastAssumingZero(WTF::Atomic<unsigned char>&) + 51 (LockAlgorithm.h:54)
2   com.apple.WebKit              	0x000000011396e959 WTF::Lock::lock() + 25 (Lock.h:59)
3   com.apple.WebKit              	0x0000000113b0d326 void WTF::addIterator<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >(WTF::HashTable<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> > const*, WTF::HashTableConstIterator<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >*) + 150 (HashTable.h:1413)
4   com.apple.WebKit              	0x0000000113b0d6db WTF::HashTableConstIterator<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >::HashTableConstIterator(WTF::HashTable<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, 
WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> > const*, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> const*, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> const*, WTF::HashItemKnownGoodTag) + 59 (HashTable.h:135)
5   com.apple.WebKit              	0x0000000113b0d695 WTF::HashTableConstIterator<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >::HashTableConstIterator(WTF::HashTable<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> > const*, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> const*, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> const*, WTF::HashItemKnownGoodTag) + 53 (HashTable.h:135)
6   com.apple.WebKit              	0x0000000113b0d653 WTF::HashTable<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >::makeKnownGoodConstIterator(WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>*) const + 67 (HashTable.h:469)
7   com.apple.WebKit              	0x0000000113b0d5af WTF::HashTable<unsigned long long, WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<unsigned long long, WebKit::WebPageProxy*> >, WTF::IntHash<unsigned long long>, WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::KeyValuePairTraits, WTF::HashTraits<unsigned long long> >::end() const + 47 (HashTable.h:380)
8   com.apple.WebKit              	0x0000000113b0d0d7 WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::end() const + 39 (HashMap.h:266)
9   com.apple.WebKit              	0x0000000113b0cebe WTF::HashMap<unsigned long long, WebKit::WebPageProxy*, WTF::IntHash<unsigned long long>, WTF::HashTraits<unsigned long long>, WTF::HashTraits<WebKit::WebPageProxy*> >::values() const + 110 (HashMap.h:106)
10  com.apple.WebKit              	0x0000000113b0cca3 WebKit::WebProcessProxy::pages() const + 35 (WebProcessProxy.h:102)
11  com.apple.WebKit              	0x0000000114556a55 WebKit::nonEphemeralWebPageProxy() + 277 (WebResourceLoadStatisticsTelemetry.cpp:138)
12  com.apple.WebKit              	0x00000001145563c1 WebKit::WebResourceLoadStatisticsTelemetry::calculateAndSubmit(WebKit::WebResourceLoadStatisticsStore const&) + 321 (WebResourceLoadStatisticsTelemetry.cpp:249)
13  com.apple.WebKit              	0x0000000114538d5d WebKit::WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore(WTF::String const&, WTF::Function<void (WTF::String const&)>&&, WTF::Function<void (WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebKit::ShouldClearFirst)>&&, WTF::Function<void (WTF::String const&, WTF::String const&, bool, WTF::Function<void (bool)>&&)>&&, WTF::Function<void (WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&)>&&)::$_1::operator()() const + 45 (WebResourceLoadStatisticsStore.cpp:171)
14  com.apple.WebKit              	0x0000000114538c99 WTF::Function<void ()>::CallableWrapper<WebKit::WebResourceLoadStatisticsStore::WebResourceLoadStatisticsStore(WTF::String const&, WTF::Function<void (WTF::String const&)>&&, WTF::Function<void (WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&, WebKit::ShouldClearFirst)>&&, WTF::Function<void (WTF::String const&, WTF::String const&, bool, WTF::Function<void (bool)>&&)>&&, WTF::Function<void (WTF::Vector<WTF::String, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc> const&)>&&)::$_1>::call() + 25 (Function.h:101)
15  com.apple.JavaScriptCore      	0x0000000110a699fb WTF::Function<void ()>::operator()() const + 139 (Function.h:56)
16  com.apple.JavaScriptCore      	0x0000000110af2169 WTF::WorkQueue::dispatchAfter(WTF::Seconds, WTF::Function<void ()>&&)::$_1::operator()() const + 25 (WorkQueueCocoa.cpp:44)
17  com.apple.JavaScriptCore      	0x0000000110af2140 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatchAfter(WTF::Seconds, WTF::Function<void ()>&&)::$_1>(WTF::WorkQueue::dispatchAfter(WTF::Seconds, WTF::Function<void ()>&&)::$_1)::'lambda'(void*)::operator()(void*) const + 32 (BlockPtr.h:94)
18  com.apple.JavaScriptCore      	0x0000000110af2118 WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatchAfter(WTF::Seconds, WTF::Function<void ()>&&)::$_1>(WTF::WorkQueue::dispatchAfter(WTF::Seconds, WTF::Function<void ()>&&)::$_1)::'lambda'(void*)::__invoke(void*) + 24 (BlockPtr.h:93)
19  libdispatch.dylib             	0x00007fff8c17e93d _dispatch_call_block_and_release + 12
20  libdispatch.dylib             	0x00007fff8c17340b _dispatch_client_callout + 8
21  libdispatch.dylib             	0x00007fff8c1861f9 _dispatch_after_timer_callback + 77
22  libdispatch.dylib             	0x00007fff8c17340b _dispatch_client_callout + 8
23  libdispatch.dylib             	0x00007fff8c183675 _dispatch_source_latch_and_call + 2235
24  libdispatch.dylib             	0x00007fff8c177a83 _dispatch_source_invoke + 983
25  libdispatch.dylib             	0x00007fff8c178200 _dispatch_queue_drain + 1207
26  libdispatch.dylib             	0x00007fff8c17e707 _dispatch_queue_invoke + 549
27  libdispatch.dylib             	0x00007fff8c176d53 _dispatch_root_queue_drain + 538
28  libdispatch.dylib             	0x00007fff8c176b00 _dispatch_worker_thread3 + 91
29  libsystem_pthread.dylib       	0x00007fff90bc34de _pthread_wqthread + 1129
30  libsystem_pthread.dylib       	0x00007fff90bc1341 start_wqthread + 13

[Ryan Haddad]

Created attachment 328864 [details]
Crash log

[Radar WebKit Bug Importer]

<rdar://problem/35942205>

[John Wilander]

Created attachment 329144 [details]
Patch

[Brent Fulgham]

Comment on attachment 329144 [details]
Patch

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

This looks right, but you are copying a bunch of stuff we can just move. Please switch to move so we don't waste energy copying these values.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:55
> +    PrevalentResourceTelemetry isolatedCopy() const

Do we need a special cross-site copy for these atomic types?

> Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:221
> +    RunLoop::main().dispatch([totalPrevalentResources = crossThreadCopy(totalPrevalentResources), totalPrevalentResourcesWithUserInteraction = crossThreadCopy(totalPrevalentResourcesWithUserInteraction), top3SubframeUnderTopFrameOrigins = crossThreadCopy(top3SubframeUnderTopFrameOrigins)] {

I just ran this by Chris, and confirmed we should be doing WTFMove() for vectors of atomic types (e.g., prevalentResourcesDaysSinceUserInteraction).

Because we never use these values agin, I think you can just WTFMove() all of them, without needing the copy.

> Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:262
> +    RunLoop::main().dispatch([sortedPrevalentResources = crossThreadCopy(sortedPrevalentResources), sortedPrevalentResourcesWithoutUserInteraction = crossThreadCopy(sortedPrevalentResourcesWithoutUserInteraction), prevalentResourcesDaysSinceUserInteraction = crossThreadCopy(prevalentResourcesDaysSinceUserInteraction)] () {

It seems like all three of these Vectors could be WTFMove() to the new thread, since they are never used again after being constructed.

[John Wilander]

Created attachment 329150 [details]
Patch

[John Wilander]

(In reply to Brent Fulgham from comment #4)
> Comment on attachment 329144 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=329144&action=review
> 
> This looks right, but you are copying a bunch of stuff we can just move.
> Please switch to move so we don't waste energy copying these values.
> 
> > Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:55
> > +    PrevalentResourceTelemetry isolatedCopy() const
> 
> Do we need a special cross-site copy for these atomic types?

We did when I was using crossThreadCopy().

> > Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:221
> > +    RunLoop::main().dispatch([totalPrevalentResources = crossThreadCopy(totalPrevalentResources), totalPrevalentResourcesWithUserInteraction = crossThreadCopy(totalPrevalentResourcesWithUserInteraction), top3SubframeUnderTopFrameOrigins = crossThreadCopy(top3SubframeUnderTopFrameOrigins)] {
> 
> I just ran this by Chris, and confirmed we should be doing WTFMove() for
> vectors of atomic types (e.g., prevalentResourcesDaysSinceUserInteraction).
> 
> Because we never use these values agin, I think you can just WTFMove() all
> of them, without needing the copy.

Got it. Fixed.

> > Source/WebKit/UIProcess/WebResourceLoadStatisticsTelemetry.cpp:262
> > +    RunLoop::main().dispatch([sortedPrevalentResources = crossThreadCopy(sortedPrevalentResources), sortedPrevalentResourcesWithoutUserInteraction = crossThreadCopy(sortedPrevalentResourcesWithoutUserInteraction), prevalentResourcesDaysSinceUserInteraction = crossThreadCopy(prevalentResourcesDaysSinceUserInteraction)] () {
> 
> It seems like all three of these Vectors could be WTFMove() to the new
> thread, since they are never used again after being constructed.

Done. Thanks, Brent!

[Brent Fulgham]

Comment on attachment 329150 [details]
Patch

Looks great! r=me

[John Wilander]

Comment on attachment 329150 [details]
Patch

Thanks again, Brent!

[WebKit Commit Bot]

Comment on attachment 329150 [details]
Patch

Clearing flags on attachment: 329150

Committed r225818: <https://trac.webkit.org/changeset/225818>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.