Changeset 205033 in webkit

Timestamp:

Aug 26, 2016, 1:22:21 PM (9 years ago)

Author:

Joseph Pecoraro

Message:

Web Inspector: HeapProfiler/ScriptProfiler do not destruct safely when JSContext is destroyed
​https://bugs.webkit.org/show_bug.cgi?id=161027
<rdar://problem/27871349>

Reviewed by Mark Lam.

For JSContext inspection, when a frontend connects keep the target alive.
This means ref'ing the JSGlobalObject / VM when the first frontend
connects and deref'ing when the last frontend disconnects.

• inspector/JSGlobalObjectInspectorController.h:
• inspector/JSGlobalObjectInspectorController.cpp:

(Inspector::JSGlobalObjectInspectorController::globalObjectDestroyed):
(Inspector::JSGlobalObjectInspectorController::disconnectAllFrontends): Deleted.
Now that frontends keep the global object alive, when the global object
is destroyed that must mean that no frontends exist. Remove the now
stale code path.

(Inspector::JSGlobalObjectInspectorController::connectFrontend):
(Inspector::JSGlobalObjectInspectorController::disconnectFrontend):
Ref the target when the first frontend connects, deref when the last disconnects.

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/JSGlobalObjectInspectorController.cpp (modified) (4 diffs)
• inspector/JSGlobalObjectInspectorController.h (modified) (4 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-08-26  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: HeapProfiler/ScriptProfiler do not destruct safely when JSContext is destroyed
+        https://bugs.webkit.org/show_bug.cgi?id=161027
+        <rdar://problem/27871349>
+
+        Reviewed by Mark Lam.
+
+        For JSContext inspection, when a frontend connects keep the target alive.
+        This means ref'ing the JSGlobalObject / VM when the first frontend
+        connects and deref'ing when the last frontend disconnects.
+
+        * inspector/JSGlobalObjectInspectorController.h:
+        * inspector/JSGlobalObjectInspectorController.cpp:
+        (Inspector::JSGlobalObjectInspectorController::globalObjectDestroyed):
+        (Inspector::JSGlobalObjectInspectorController::disconnectAllFrontends): Deleted.
+        Now that frontends keep the global object alive, when the global object
+        is destroyed that must mean that no frontends exist. Remove the now
+        stale code path.
+
+        (Inspector::JSGlobalObjectInspectorController::connectFrontend):
+        (Inspector::JSGlobalObjectInspectorController::disconnectFrontend):
+        Ref the target when the first frontend connects, deref when the last disconnects.
+
 2016-08-26  Yusuke Suzuki  <utatane.tea@gmail.com>
 

trunk/Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.cpp

@@ -116 +116 @@
 void JSGlobalObjectInspectorController::globalObjectDestroyed()
 {
-    disconnectAllFrontends();
+    ASSERT(!m_frontendRouter->hasFrontends());
 
     m_injectedScriptManager->disconnect();
@@ -132 +132 @@
     if (!connectedFirstFrontend)
         return;
+
+    // Keep the JSGlobalObject and VM alive while we are debugging it.
+    m_strongVM = &m_globalObject.vm();
+    m_strongGlobalObject.set(m_globalObject.vm(), &m_globalObject);
 
     // FIXME: change this to notify agents which frontend has connected (by id).
@@ -163 +167 @@
         m_augmentingClient->inspectorDisconnected();
 #endif
-}
-
-void JSGlobalObjectInspectorController::disconnectAllFrontends()
-{
-    // FIXME: change this to notify agents which frontend has disconnected (by id).
-    m_agents.willDestroyFrontendAndBackend(DisconnectReason::InspectedTargetDestroyed);
-
-    m_frontendRouter->disconnectAllFrontends();
-
-    m_isAutomaticInspection = false;
-
-#if ENABLE(INSPECTOR_ALTERNATE_DISPATCHERS)
-    if (m_augmentingClient)
-        m_augmentingClient->inspectorDisconnected();
-#endif
+
+    // Remove our JSGlobalObject and VM references, we are done debugging it.
+    m_strongGlobalObject.clear();
+    m_strongVM = nullptr;
 }
 
@@ -316 +309 @@
 
 } // namespace Inspector
-

trunk/Source/JavaScriptCore/inspector/JSGlobalObjectInspectorController.h

@@ -43 +43 @@
 class ExecState;
 class JSGlobalObject;
-class JSValue;
 }
 
@@ -54 +53 @@
 class InspectorConsoleAgent;
 class InspectorDebuggerAgent;
-class InspectorHeapAgent;
 class InspectorScriptProfilerAgent;
 class JSGlobalObjectConsoleClient;
@@ -73 +71 @@
     void connectFrontend(FrontendChannel*, bool isAutomaticInspection);
     void disconnectFrontend(FrontendChannel*);
-    void disconnectAllFrontends();
 
     void dispatchMessageFromFrontend(const String&);
@@ -122 +119 @@
     Ref<BackendDispatcher> m_backendDispatcher;
 
+    // Used to keep the JSGlobalObject and VM alive while we are debugging it.
+    JSC::Strong<JSC::JSGlobalObject> m_strongGlobalObject;
+    RefPtr<JSC::VM> m_strongVM;
+
     bool m_includeNativeCallStackWithExceptions { true };
     bool m_isAutomaticInspection { false };