Bug 217111

Summary: Crash while loading a confluence page
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: DOMAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED INVALID    
Severity: Normal CC: cdumez, commit-queue, darin, esprehn+autocc, ews-watchlist, kangil.han, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 217923    
Bug Blocks:    
Attachments:
Description Flags
Fixes the bug
none
Patch for landing
ews-feeder: commit-queue-
Fixes the bug none

Description Ryosuke Niwa 2020-09-29 17:48:57 PDT 
We can hit the following crash while loading a confluence page:

./dom/Node.cpp(2293) : void WebCore::Node::unregisterMutationObserver(WebCore::MutationObserverRegistration &)
1   0x6ec0debd9 WTFCrash
2   0x6cdf3b03b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x6d0d4f4bd WebCore::Node::unregisterMutationObserver(WebCore::MutationObserverRegistration&)
4   0x6d0d1cfb6 WebCore::MutationObserver::disconnect()
5   0x6cef7e128 WebCore::jsMutationObserverPrototypeFunctionDisconnectBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSMutationObserver*)
6   0x6ceef0c6c long long WebCore::IDLOperation<WebCore::JSMutationObserver>::call<&(WebCore::jsMutationObserverPrototypeFunctionDisconnectBody(JSC::JSGlobalObject*, JSC::CallFrame*, WebCore::JSMutationObserver*)), (WebCore::CastedThisErrorBehavior)0>(JSC::JSGlobalObject&, JSC::CallFrame&, char const*)
7   0x6ceef0944 WebCore::jsMutationObserverPrototypeFunctionDisconnect(JSC::JSGlobalObject*, JSC::CallFrame*)
8   0x30ae70801178
9   0x6ec65539d llint_entry
10  0x6ec632543 vmEntryToJavaScript
11  0x6ed4668fb JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
12  0x6ed465e12 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::JSGlobalObject*, JSC::JSObject*)
13  0x6ed810937 JSC::evaluate(JSC::JSGlobalObject*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
14  0x6ed810a8a JSC::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
15  0x6d06a8e9c WebCore::JSExecState::profiledEvaluate(JSC::JSGlobalObject*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
16  0x6d06a8a7e WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
17  0x6d06a88a9 WebCore::ScriptController::evaluateInWorldIgnoringException(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&)
18  0x6d06a91a5 WebCore::ScriptController::evaluateIgnoringException(WebCore::ScriptSourceCode const&)
19  0x6d0d9d6b6 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&)
20  0x6d0d9b62b WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport)
21  0x6d1319216 WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&)
22  0x6d1319017 WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::DumbPtrTraits<WebCore::ScriptElement> >&&, WTF::TextPosition const&)
23  0x6d12fd721 WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder()
24  0x6d12fdbac WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&)
25  0x6d12fcf1f WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode)
26  0x6d12fc86d WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode)
27  0x6d12fe972 WebCore::HTMLDocumentParser::append(WTF::RefPtr<WTF::StringImpl, WTF::DumbPtrTraits<WTF::StringImpl>, WTF::DefaultRefDerefTraits<WTF::StringImpl> >&&)
28  0x6d0b96616 WebCore::DecodedDataDocumentParser::flush(WebCore::DocumentWriter&)
29  0x6d17090fc WebCore::DocumentWriter::end()
30  0x6d1708144 WebCore::DocumentLoader::finishedLoading()
31  0x6d1707b41 WebCore::DocumentLoader::notifyFinished(WebCore::CachedResource&, WebCore::NetworkLoadMetrics const&)

<rdar://problem/69757679>
Comment 1 Ryosuke Niwa 2020-09-29 17:54:22 PDT 
Created attachment 410076 [details]
Fixes the bug
Comment 2 Ryosuke Niwa 2020-09-29 17:54:52 PDT 
Comment on attachment 410076 [details]
Fixes the bug

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

> Source/WebCore/dom/MutationObserverRegistration.h:72
> -    std::unique_ptr<HashSet<GCReachableRef<Node>>> m_transientRegistrationNodes;
> +    bool m_hasStoppedObservingNode { false };

This re-ordering helps with packing.
Comment 3 Wenson Hsieh 2020-09-29 18:21:37 PDT 
Comment on attachment 410076 [details]
Fixes the bug

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

> Source/WebCore/dom/MutationObserver.cpp:124
> +        auto node = registration->takeNodeAndStopObserving();

Nit - usually, methods like `takeFoo()` also remove `Foo` from the object.

(We chatted on slack and decided to go with just `stopObserving()` instead)
Comment 4 Ryosuke Niwa 2020-09-29 18:22:27 PDT 
Created attachment 410079 [details]
Patch for landing
Comment 5 Ryosuke Niwa 2020-09-29 18:22:59 PDT 
Comment on attachment 410079 [details]
Patch for landing

Let's wait for EWS.
Comment 6 Ryosuke Niwa 2020-09-29 18:26:10 PDT 
Created attachment 410082 [details]
Fixes the bug
Comment 7 Ryosuke Niwa 2020-09-29 18:27:38 PDT 
Comment on attachment 410082 [details]
Fixes the bug

Waiting for EWS.
Comment 8 EWS 2020-09-29 18:54:39 PDT 
Patch 410079 does not build
Comment 9 Ryosuke Niwa 2020-09-29 20:17:47 PDT 
Comment on attachment 410082 [details]
Fixes the bug

Clearing flags on attachment: 410082

Committed r267779: <https://trac.webkit.org/changeset/267779>
Comment 10 Ryosuke Niwa 2020-09-29 20:17:49 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2020-09-29 20:18:21 PDT 
<rdar://problem/69775129>
Comment 12 WebKit Commit Bot 2020-10-19 15:08:58 PDT 
Re-opened since this is blocked by bug 217923