Changeset 248376 in webkit

Timestamp:

Aug 7, 2019 10:31:10 AM (5 years ago)

Author:

Chris Dumez

Message:

Fix thread safety issue under JSHistory::visitAdditionalChildren()
​https://bugs.webkit.org/show_bug.cgi?id=200504

Reviewed by Darin Adler.

JSHistory::visitAdditionalChildren() is called from a GC thread and was calling
History::cachedState() whose implementation goes deep into WebCore
(FrameLoader / HistoryController). Among other things, it null checks m_window
which is a WeakPtr and then later dereferences it to get the Frame object, which
is not safe from the non-main thread.

To address the issue, introduce a simpler cachedStateForGC() getter which is
thread safe, and use it in JSHistory::visitAdditionalChildren().

• bindings/js/JSHistoryCustom.cpp:

(WebCore::JSHistory::visitAdditionalChildren):

• page/History.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/JSHistoryCustom.cpp (modified) (1 diff)
• page/History.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-07  Chris Dumez  <cdumez@apple.com>
+
+        Fix thread safety issue under JSHistory::visitAdditionalChildren()
+        https://bugs.webkit.org/show_bug.cgi?id=200504
+
+        Reviewed by Darin Adler.
+
+        JSHistory::visitAdditionalChildren() is called from a GC thread and was calling
+        History::cachedState() whose implementation goes deep into WebCore
+        (FrameLoader / HistoryController). Among other things, it null checks m_window
+        which is a WeakPtr and then later dereferences it to get the Frame object, which
+        is not safe from the non-main thread.
+
+        To address the issue, introduce a simpler cachedStateForGC() getter which is
+        thread safe, and use it in JSHistory::visitAdditionalChildren().
+
+        * bindings/js/JSHistoryCustom.cpp:
+        (WebCore::JSHistory::visitAdditionalChildren):
+        * page/History.h:
+
 2019-08-07  Priyanka Agarwal  <pagarwal999@apple.com>
 

trunk/Source/WebCore/bindings/js/JSHistoryCustom.cpp

@@ -47 +47 @@
 void JSHistory::visitAdditionalChildren(SlotVisitor& visitor)
 {
-    wrapped().cachedState().visit(visitor);
+    wrapped().cachedStateForGC().visit(visitor);
 }
 

trunk/Source/WebCore/page/History.h

@@ -54 +54 @@
     SerializedScriptValue* state();
     JSValueInWrappedObject& cachedState();
+    JSValueInWrappedObject& cachedStateForGC() { return m_cachedState; }
 
     void back();