Bug 209024

Summary: [ Mac wk2 Debug ] webgpu/whlsl/vector-compare.html is flaky crashing.
Product: WebKit Reporter: Jason Lawrence <Lawrence.j>
Component: WebGPUAssignee: Justin Fan <justin_fan>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dino, ews-watchlist, graouts, justin_fan, kondapallykalyan, mmaxfield, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: macOS 10.15   
Attachments:
Description Flags
vector-compare-crash-log
none
Patch
none
Patch
none
Patch for landing none

Description Jason Lawrence 2020-03-12 16:09:35 PDT 
Created attachment 393426 [details]
vector-compare-crash-log

webgpu/whlsl/vector-compare.html

Description:
This test is flaky crashing on Mac wk2 Debug. The flaky crashing is present through nearly all of the visible history.

History:
https://results.webkit.org/?suite=layout-tests&test=webgpu%2Fwhlsl%2Fvector-compare.html&recent=false&style=debug&limit=50000

Crash log attached:
Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x00000002f1e1341e WTFCrash + 14 (Assertions.cpp:305)
1   com.apple.WebCore             	0x00000002d517347b WTFCrashWithInfo(int, char const*, char const*, int) + 27
2   com.apple.WebCore             	0x00000002d51867a8 WTF::RefCountedBase::hasOneRef() const + 104 (RefCounted.h:55)
3   com.apple.WebCore             	0x00000002d69d56af WebCore::GPUBindGroupAllocator::tryReset() + 31 (GPUBindGroupAllocatorMetal.mm:156)
4   com.apple.WebCore             	0x00000002d69d865b WebCore::GPUBindGroup::~GPUBindGroup() + 75 (GPUBindGroupMetal.mm:244)
5   com.apple.WebCore             	0x00000002d69d87f5 WebCore::GPUBindGroup::~GPUBindGroup() + 21 (GPUBindGroupMetal.mm:245)
6   com.apple.WebCore             	0x00000002d660003b std::__1::default_delete<WebCore::GPUBindGroup>::operator()(WebCore::GPUBindGroup*) const + 43 (memory:2339)
7   com.apple.WebCore             	0x00000002d65ffffc WTF::RefCounted<WebCore::GPUBindGroup, std::__1::default_delete<WebCore::GPUBindGroup> >::deref() const + 60 (RefCounted.h:191)
8   com.apple.WebCore             	0x00000002d65fff81 void WTF::derefIfNotNull<WebCore::GPUBindGroup>(WebCore::GPUBindGroup*) + 49 (RefPtr.h:45)
9   com.apple.WebCore             	0x00000002d65fff49 WTF::RefPtr<WebCore::GPUBindGroup, WTF::DumbPtrTraits<WebCore::GPUBindGroup> >::~RefPtr() + 41 (RefPtr.h:70)
10  com.apple.WebCore             	0x00000002d65ffef5 WTF::RefPtr<WebCore::GPUBindGroup, WTF::DumbPtrTraits<WebCore::GPUBindGroup> >::~RefPtr() + 21 (RefPtr.h:70)
11  com.apple.WebCore             	0x00000002d65ffec3 WebCore::WebGPUBindGroup::~WebGPUBindGroup() + 35 (WebGPUBindGroup.h:36)
12  com.apple.WebCore             	0x00000002d65ffe75 WebCore::WebGPUBindGroup::~WebGPUBindGroup() + 21 (WebGPUBindGroup.h:36)
Comment 1 Jason Lawrence 2020-03-12 16:15:25 PDT 
I have marked this test as crashing while this issue is investigated here: https://trac.webkit.org/changeset/258368/webkit

This issue is also being tracked here: <rdar://problem/57304995>
Comment 2 Alexey Proskuryakov 2020-03-12 16:19:26 PDT 
That's a different issue according to the call stack. Importing as a new one.
Comment 3 Radar WebKit Bug Importer 2020-03-12 16:19:35 PDT 
<rdar://problem/60392533>
Comment 4 Justin Fan 2020-03-13 18:33:53 PDT 
Created attachment 393568 [details]
Patch
Comment 5 Darin Adler 2020-03-15 17:28:12 PDT 
Comment on attachment 393568 [details]
Patch

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

> Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupAllocatorMetal.mm:160
> +    // This method is called in ~GPUBindGroup(). A refCount of 2 means either only the GPUDevice and the
> +    // lone remaining GPUBindGroup reference this allocator, in which case a reset can occur,
> +    // or the GPUDevice has already been cleaned up and the 2 remaining GPUBindGroups cannot be used again anyway.
> +    if (refCount() != 2)
>          return;

This seems super-fragile.

If you change this 2 to a 1 does a test fail? Which test?

If you change this 2 to a 3 does a test fail? Which test?
Comment 6 Justin Fan 2020-03-23 16:43:54 PDT 
Created attachment 394326 [details]
Patch
Comment 7 Myles C. Maxfield 2020-03-23 17:39:56 PDT 
Comment on attachment 394326 [details]
Patch

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

> Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupMetal.mm:242
> +    m_allocator->tryReset();

Can't you do this with a weak pointer instead?
Comment 8 Justin Fan 2020-03-23 18:51:20 PDT 
Created attachment 394339 [details]
Patch for landing
Comment 9 EWS 2020-03-23 19:38:45 PDT 
Committed r258899: <https://trac.webkit.org/changeset/258899>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 394339 [details].