Bug 164715 – REGRESSION (r208606?): LayoutTest fast/mediastream/enumerating-crash.html is a flaky crash

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

[Ryan Haddad]

LayouTest fast/mediastream/enumerating-crash.html is a flaky crash

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

[Ryan Haddad]

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   com.apple.JavaScriptCore      	0x000000010adcd4f7 WTFCrash + 39 (Assertions.cpp:323)
1   com.apple.JavaScriptCore      	0x000000010adcd519 WTFCrashWithSecurityImplication + 9
2   com.apple.WebKit              	0x00000001067a8ecf WTF::match_constness<WebCore::ScriptExecutionContext, WebCore::Document>::type* WTF::downcast<WebCore::Document, WebCore::ScriptExecutionContext>(WebCore::ScriptExecutionContext*) + 79 (TypeCasts.h:89)
3   com.apple.WebKit              	0x00000001067a826d WebCore::UserMediaRequest::document() const + 29 (UserMediaRequest.h:74)
4   com.apple.WebKit              	0x00000001067a77c8 WebKit::UserMediaPermissionRequestManager::removeMediaRequestFromMaps(WebCore::UserMediaRequest&) + 56 (UserMediaPermissionRequestManager.cpp:126)
5   com.apple.WebKit              	0x00000001067a777b WebKit::UserMediaPermissionRequestManager::cancelUserMediaRequest(WebCore::UserMediaRequest&) + 107 (UserMediaPermissionRequestManager.cpp:108)
6   com.apple.WebKit              	0x0000000106cefe39 WebKit::WebUserMediaClient::cancelUserMediaAccessRequest(WebCore::UserMediaRequest&) + 41 (WebUserMediaClient.cpp:51)
7   com.apple.WebCore             	0x000000010fe366a3 WebCore::UserMediaController::cancelUserMediaAccessRequest(WebCore::UserMediaRequest&) + 51 (UserMediaController.h:64)
8   com.apple.WebCore             	0x000000010fe34386 WebCore::UserMediaRequest::contextDestroyed() + 70 (UserMediaRequest.cpp:235)
9   com.apple.WebCore             	0x000000010f864450 WebCore::ScriptExecutionContext::~ScriptExecutionContext() + 144 (ScriptExecutionContext.cpp:127)
10  com.apple.WebCore             	0x000000010db1ace0 WebCore::Document::~Document() + 8320 (Document.cpp:656)
11  com.apple.WebCore             	0x000000010e074395 WebCore::HTMLDocument::~HTMLDocument() + 117 (HTMLDocument.cpp:93)
12  com.apple.WebCore             	0x000000010e0743b5 WebCore::HTMLDocument::~HTMLDocument() + 21 (HTMLDocument.cpp:93)
13  com.apple.WebCore             	0x000000010e074439 WebCore::HTMLDocument::~HTMLDocument() + 25 (HTMLDocument.cpp:92)
14  com.apple.WebCore             	0x000000010db3d31d WebCore::Document::decrementReferencingNodeCount() + 189 (Document.h:324)
15  com.apple.WebCore             	0x000000010f2721c1 WebCore::Node::~Node() + 737 (Node.cpp:310)
16  com.apple.WebCore             	0x000000010d75ea49 WebCore::ContainerNode::~ContainerNode() + 105 (ContainerNode.cpp:156)
17  com.apple.WebCore             	0x000000010dca876f WebCore::Element::~Element() + 495 (Element.cpp:203)
18  com.apple.WebCore             	0x000000010facf7c7 WebCore::StyledElement::~StyledElement() + 87 (StyledElement.cpp:139)
19  com.apple.WebCore             	0x000000010d583c35 WebCore::HTMLElement::~HTMLElement() + 21 (HTMLElement.h:38)
20  com.apple.WebCore             	0x000000010e169265 WebCore::HTMLSpanElement::~HTMLSpanElement() + 21 (HTMLSpanElement.h:32)
21  com.apple.WebCore             	0x000000010e1691c5 WebCore::HTMLSpanElement::~HTMLSpanElement() + 21 (HTMLSpanElement.h:32)
22  com.apple.WebCore             	0x000000010e1691e9 WebCore::HTMLSpanElement::~HTMLSpanElement() + 25 (HTMLSpanElement.h:32)
23  com.apple.WebCore             	0x000000010f27b94d WebCore::Node::removedLastRef() + 93 (Node.cpp:2327)
24  com.apple.WebCore             	0x000000010d3fa8ae WebCore::Node::deref() + 382 (Node.h:724)
25  com.apple.WebCore             	0x000000010f2747f5 WebCore::Node::derefEventTarget() + 21 (Node.cpp:750)
26  com.apple.WebCore             	0x000000010db794d6 WebCore::EventTarget::deref() + 22 (EventTarget.h:65)
27  com.apple.WebCore             	0x000000010dc444bd WTF::Ref<WebCore::EventTarget>::~Ref() + 45 (Ref.h:60)
28  com.apple.WebCore             	0x000000010dc36805 WTF::Ref<WebCore::EventTarget>::~Ref() + 21 (Ref.h:60)
29  com.apple.WebCore             	0x000000010e803dcc WebCore::JSDOMWrapper<WebCore::EventTarget>::~JSDOMWrapper() + 28 (JSDOMWrapper.h:76)
30  com.apple.WebCore             	0x000000010e803da5 WebCore::JSEventTarget::~JSEventTarget() + 21 (JSEventTarget.h:30)
31  com.apple.WebCore             	0x000000010e8037f5 WebCore::JSEventTarget::~JSEventTarget() + 21 (JSEventTarget.h:30)
32  com.apple.WebCore             	0x000000010e801cbd WebCore::JSEventTarget::destroy(JSC::JSCell*) + 29 (JSEventTarget.cpp:197)
33  com.apple.JavaScriptCore      	0x0000000109bbc12b JSC::JSCell::callDestructor(JSC::VM&) + 203 (JSCellInlines.h:291)
34  com.apple.JavaScriptCore      	0x000000010a985942 JSC::FreeList JSC::MarkedBlock::Handle::specializedSweep<(JSC::MarkedBlock::Handle::EmptyMode)1, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::DestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)1, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)1, (JSC::MarkedBlock::Handle::MarksMode)1>() + 306 (MarkedBlock.cpp:126)
35  com.apple.JavaScriptCore      	0x000000010a984fe3 JSC::FreeList JSC::MarkedBlock::Handle::sweepHelperSelectMarksMode<(JSC::MarkedBlock::Handle::EmptyMode)1, (JSC::MarkedBlock::Handle::SweepMode)0, (JSC::DestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)1, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)1>() + 83 (MarkedBlock.cpp:230)
36  com.apple.JavaScriptCore      	0x000000010a98431e JSC::FreeList JSC::MarkedBlock::Handle::sweepHelperSelectSweepMode<(JSC::MarkedBlock::Handle::EmptyMode)1, (JSC::DestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)1, (JSC::MarkedBlock::Handle::NewlyAllocatedMode)1>(JSC::MarkedBlock::Handle::SweepMode) + 78 (MarkedBlock.cpp:222)
37  com.apple.JavaScriptCore      	0x000000010a982cec JSC::FreeList JSC::MarkedBlock::Handle::sweepHelperSelectHasNewlyAllocated<(JSC::MarkedBlock::Handle::EmptyMode)1, (JSC::DestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)1>(JSC::MarkedBlock::Handle::SweepMode) + 92 (MarkedBlock.cpp:214)
38  com.apple.JavaScriptCore      	0x000000010a982ba3 JSC::FreeList JSC::MarkedBlock::Handle::sweepHelperSelectEmptyMode<(JSC::DestructionMode)1, (JSC::MarkedBlock::Handle::ScribbleMode)1>(JSC::MarkedBlock::Handle::SweepMode) + 99 (MarkedBlock.cpp:206)
39  com.apple.JavaScriptCore      	0x000000010a9822b4 JSC::FreeList JSC::MarkedBlock::Handle::sweepHelperSelectScribbleMode<(JSC::DestructionMode)1>(JSC::MarkedBlock::Handle::SweepMode) + 68 (MarkedBlock.cpp:189)
40  com.apple.JavaScriptCore      	0x000000010a9810d5 JSC::MarkedBlock::Handle::sweep(JSC::MarkedBlock::Handle::SweepMode) + 341 (MarkedBlock.cpp:181)
41  com.apple.JavaScriptCore      	0x000000010a5ebeeb JSC::IncrementalSweeper::sweepNextBlock() + 187 (IncrementalSweeper.cpp:87)
42  com.apple.JavaScriptCore      	0x000000010a5ebdd2 JSC::IncrementalSweeper::doSweep(double) + 34 (IncrementalSweeper.cpp:60)
43  com.apple.JavaScriptCore      	0x000000010a5ebda2 JSC::IncrementalSweeper::doWork() + 34 (IncrementalSweeper.cpp:56)
44  com.apple.JavaScriptCore      	0x000000010a5e81e7 JSC::HeapTimer::timerDidFire(__CFRunLoopTimer*, void*) + 183 (HeapTimer.cpp:93)
45  com.apple.CoreFoundation      	0x00007fff8997fb94 __CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__ + 20
46  com.apple.CoreFoundation      	0x00007fff8997f823 __CFRunLoopDoTimer + 1075
47  com.apple.CoreFoundation      	0x00007fff8997f37a __CFRunLoopDoTimers + 298
48  com.apple.CoreFoundation      	0x00007fff89976871 __CFRunLoopRun + 1841
49  com.apple.CoreFoundation      	0x00007fff89975ed8 CFRunLoopRunSpecific + 296
50  com.apple.HIToolbox           	0x00007fff8b2be935 RunCurrentEventLoopInMode + 235
51  com.apple.HIToolbox           	0x00007fff8b2be76f ReceiveNextEventCommon + 432
52  com.apple.HIToolbox           	0x00007fff8b2be5af _BlockUntilNextEventMatchingListInModeWithFilter + 71
53  com.apple.AppKit              	0x00007fff90186df6 _DPSNextEvent + 1067
54  com.apple.AppKit              	0x00007fff90186226 -[NSApplication _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 454
55  com.apple.AppKit              	0x00007fff9017ad80 -[NSApplication run] + 682
56  com.apple.AppKit              	0x00007fff90144368 NSApplicationMain + 1176
57  libxpc.dylib                  	0x00007fff94c0c194 _xpc_objc_main + 795
58  libxpc.dylib                  	0x00007fff94c0abbe xpc_main + 494
59  com.apple.WebKit.WebContent   	0x0000000106260080 main + 800
60  libdyld.dylib                 	0x00007fff824d65ad start + 1

[Ryan Haddad]

Related to http://trac.webkit.org/projects/webkit/changeset/208606?

[Eric Carlson]

Created attachment 294804 [details]
Proposed patch.

[WebKit Commit Bot]

Comment on attachment 294804 [details]
Proposed patch.

Clearing flags on attachment: 294804

Committed r208730: <http://trac.webkit.org/changeset/208730>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

This is still happening, just saw this crash with ToT.

[Ryan Haddad]

(In reply to comment #6)
> This is still happening, just saw this crash with ToT.

Seen with fast/regions/cssom/webkit-named-flow-event-target.html here:

https://build.webkit.org/results/Apple%20Yosemite%20Debug%20WK2%20(Tests)/r208748%20(16238)/results.html

[Alexey Proskuryakov]

<rdar://problem/29277180>

[Eric Carlson]

Created attachment 295125 [details]
Proposed patch.

[Alexey Proskuryakov]

Comment on attachment 295125 [details]
Proposed patch.

r=me.

Looking at the code, I wonder if there is a mistake in ScriptExecutionContext here:

    virtual bool isDocument() const { return false; }
    virtual bool isWorkerGlobalScope() const { return false; }

Don't we want to ASSERT_NOT_REACHED here, or just to leave it unimplemented?

[Chris Dumez]

(In reply to comment #10)
> Comment on attachment 295125 [details]
> Proposed patch.
> 
> r=me.
> 
> Looking at the code, I wonder if there is a mistake in
> ScriptExecutionContext here:
> 
>     virtual bool isDocument() const { return false; }
>     virtual bool isWorkerGlobalScope() const { return false; }
> 
> Don't we want to ASSERT_NOT_REACHED here, or just to leave it unimplemented?

If you use ASSERT_NOT_REACHED() here or make them pure virtual then each subclass will have to overwrite both of this virtual functions. Currently, each subclass only has to override one of them, which is nice.

This seems to be a common pattern in WebKit. I guess here you could make the argument that there are only 2 virtual functions and it is not a big deal to override all of them in each subclass. However, if you consider other base classes like Node or Element, this does not scale very well.

[WebKit Commit Bot]

Comment on attachment 295125 [details]
Proposed patch.

Clearing flags on attachment: 295125

Committed r208938: <http://trac.webkit.org/changeset/208938>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.