Changeset 183178 in webkit

Timestamp:

Apr 23, 2015, 12:57:57 AM (10 years ago)

Author:

Antti Koivisto

Message:

CrashTracer: WebProcess at com.apple.WebCore: WebCore::toScriptElementIfPossible + 4
​https://bugs.webkit.org/show_bug.cgi?id=144050
rdar://problem/15534973

Reviewed by Chris Dumez.

We are seeing null Element pointer crashes with this stack:

47 com.apple.WebCore: WebCore::toScriptElementIfPossible + 4 <==
47 com.apple.WebCore: WebCore::ScriptRunner::timerFired + 452
47 com.apple.WebCore: WebCore::ThreadTimers::sharedTimerFiredInternal + 175

The most likely cause seems to be that this code

ASSERT(m_pendingAsyncScripts.contains(scriptElement));
m_scriptsToExecuteSoon.append(m_pendingAsyncScripts.take(scriptElement));

in ScriptRunner::notifyScriptReady fails to find scriptElement and we are left with a null entry in
m_scriptsToExecuteSoon. However I haven't managed to repro this or find the exact path how this
could happen. The related code is fragile with lot of state (in ScriptElement class)
and involves many opportunities for re-entry via scripts.

No repro, no test case.

• dom/ScriptRunner.cpp:

(WebCore::ScriptRunner::timerFired):

Paper this over by adding a null check. We could check m_pendingAsyncScripts.take() above
but this also covers possibility this is caused by something else.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/ScriptRunner.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-04-22  Antti Koivisto  <antti@apple.com>
+
+        CrashTracer: WebProcess at com.apple.WebCore: WebCore::toScriptElementIfPossible + 4
+        https://bugs.webkit.org/show_bug.cgi?id=144050
+        rdar://problem/15534973
+
+        Reviewed by Chris Dumez.
+
+        We are seeing null Element pointer crashes with this stack:
+
+        47 com.apple.WebCore:  WebCore::toScriptElementIfPossible + 4 <==
+        47 com.apple.WebCore:  WebCore::ScriptRunner::timerFired + 452
+        47 com.apple.WebCore:  WebCore::ThreadTimers::sharedTimerFiredInternal + 175
+
+        The most likely cause seems to be that this code
+
+            ASSERT(m_pendingAsyncScripts.contains(scriptElement));
+            m_scriptsToExecuteSoon.append(m_pendingAsyncScripts.take(scriptElement));
+
+        in ScriptRunner::notifyScriptReady fails to find scriptElement and we are left with a null entry in
+        m_scriptsToExecuteSoon. However I haven't managed to repro this or find the exact path how this
+        could happen. The related code is fragile with lot of state (in ScriptElement class)
+        and involves many opportunities for re-entry via scripts.
+
+        No repro, no test case.
+
+        * dom/ScriptRunner.cpp:
+        (WebCore::ScriptRunner::timerFired):
+
+            Paper this over by adding a null check. We could check m_pendingAsyncScripts.take() above
+            but this also covers possibility this is caused by something else.
+
 2015-04-23  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/dom/ScriptRunner.cpp

@@ -115 +115 @@
         CachedScript* cachedScript = scripts[i].cachedScript();
         RefPtr<Element> element = scripts[i].releaseElementAndClear();
+        ASSERT(element);
+        // Paper over https://bugs.webkit.org/show_bug.cgi?id=144050
+        if (!element)
+            continue;
         toScriptElementIfPossible(element.get())->execute(cachedScript);
         m_document.decrementLoadEventDelayCount();