Title: [162735] trunk/Source
Revision
162735
Author
mark....@apple.com
Date
2014-01-24 15:44:50 -0800 (Fri, 24 Jan 2014)

Log Message

ASSERT(!m_markedSpace.m_currentDelayedReleaseScope) reloading page in inspector.
<https://webkit.org/b/127582>

Reviewed by Mark Hahnenberg.

Source/_javascript_Core: 

1. We should not enter a HeapIterationScope when we iterate the CodeBlocks.
   Apparently, iterating the CodeBlocks does not count as heap iteration.

2. If we're detaching the debugger due to the JSGlobalObject destructing,
   then we don't need to clear the debugger requests in the associated
   CodeBlocks. The JSGlobalObject destructing would mean that those
   CodeBlocks would be destructing too, and it may not be safe to access
   them anyway at this point.

The assertion failure is because we had entered a HeapIterationScope
while the JSGlobalObject is destructing, which in turn means that GC
sweeping is in progress. It's not legal to iterate the heap while the GC
is sweeping. Once we fixed the above 2 issues, we will no longer have
the conditions that manifests this assertion failure.

* debugger/Debugger.cpp:
(JSC::Debugger::detach):
(JSC::Debugger::setSteppingMode):
(JSC::Debugger::toggleBreakpoint):
(JSC::Debugger::clearBreakpoints):
(JSC::Debugger::clearDebuggerRequests):
* debugger/Debugger.h:
* runtime/JSGlobalObject.cpp:
(JSC::JSGlobalObject::~JSGlobalObject):

Source/WebCore: 

No new tests.

* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::attachDebugger):
* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::detachDebugger):
- Adding reasons for detaching a globalObject from the debugger.

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (162734 => 162735)


--- trunk/Source/_javascript_Core/ChangeLog	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/_javascript_Core/ChangeLog	2014-01-24 23:44:50 UTC (rev 162735)
@@ -1,3 +1,35 @@
+2014-01-24  Mark Lam  <mark....@apple.com>
+
+        ASSERT(!m_markedSpace.m_currentDelayedReleaseScope) reloading page in inspector.
+        <https://webkit.org/b/127582>
+
+        Reviewed by Mark Hahnenberg.
+
+        1. We should not enter a HeapIterationScope when we iterate the CodeBlocks.
+           Apparently, iterating the CodeBlocks does not count as heap iteration.
+
+        2. If we're detaching the debugger due to the JSGlobalObject destructing,
+           then we don't need to clear the debugger requests in the associated
+           CodeBlocks. The JSGlobalObject destructing would mean that those
+           CodeBlocks would be destructing too, and it may not be safe to access
+           them anyway at this point.
+
+        The assertion failure is because we had entered a HeapIterationScope
+        while the JSGlobalObject is destructing, which in turn means that GC
+        sweeping is in progress. It's not legal to iterate the heap while the GC
+        is sweeping. Once we fixed the above 2 issues, we will no longer have
+        the conditions that manifests this assertion failure.
+
+        * debugger/Debugger.cpp:
+        (JSC::Debugger::detach):
+        (JSC::Debugger::setSteppingMode):
+        (JSC::Debugger::toggleBreakpoint):
+        (JSC::Debugger::clearBreakpoints):
+        (JSC::Debugger::clearDebuggerRequests):
+        * debugger/Debugger.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::~JSGlobalObject):
+
 2014-01-24  Brent Fulgham  <bfulg...@apple.com>
 
         [Win] Convert some NMake files to MSBuild project files

Modified: trunk/Source/_javascript_Core/debugger/Debugger.cpp (162734 => 162735)


--- trunk/Source/_javascript_Core/debugger/Debugger.cpp	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/_javascript_Core/debugger/Debugger.cpp	2014-01-24 23:44:50 UTC (rev 162735)
@@ -176,7 +176,7 @@
     m_globalObjects.add(globalObject);
 }
 
-void Debugger::detach(JSGlobalObject* globalObject)
+void Debugger::detach(JSGlobalObject* globalObject, ReasonForDetach reason)
 {
     // If we're detaching from the currently executing global object, manually tear down our
     // stack, since we won't get further debugger callbacks to do so. Also, resume execution,
@@ -190,7 +190,12 @@
     ASSERT(m_globalObjects.contains(globalObject));
     m_globalObjects.remove(globalObject);
 
-    clearDebuggerRequests(globalObject);
+    // If the globalObject is destructing, then its CodeBlocks will also be
+    // destructed. There is no need to do the debugger requests clean up, and
+    // it is not safe to access those CodeBlocks at this time anyway.
+    if (reason != GlobalObjectIsDestructing)
+        clearDebuggerRequests(globalObject);
+
     globalObject->setDebugger(0);
     if (!m_globalObjects.size())
         m_vm = nullptr;
@@ -228,7 +233,6 @@
 
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     SetSteppingModeFunctor functor(this, mode);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -313,7 +317,6 @@
 {
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     ToggleBreakpointFunctor functor(this, breakpoint, enabledOrNot);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -493,7 +496,6 @@
 
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     ClearCodeBlockDebuggerRequestsFunctor functor(this);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -519,7 +521,6 @@
 void Debugger::clearDebuggerRequests(JSGlobalObject* globalObject)
 {
     ASSERT(m_vm);
-    HeapIterationScope iterationScope(m_vm->heap);
     ClearDebuggerRequestsFunctor functor(globalObject);
     m_vm->heap.forEachCodeBlock(functor);
 }

Modified: trunk/Source/_javascript_Core/debugger/Debugger.h (162734 => 162735)


--- trunk/Source/_javascript_Core/debugger/Debugger.h	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/_javascript_Core/debugger/Debugger.h	2014-01-24 23:44:50 UTC (rev 162735)
@@ -61,7 +61,11 @@
     bool needsExceptionCallbacks() const { return m_pauseOnExceptionsState != DontPauseOnExceptions; }
 
     void attach(JSGlobalObject*);
-    virtual void detach(JSGlobalObject*);
+    enum ReasonForDetach {
+        TerminatingDebuggingSession,
+        GlobalObjectIsDestructing
+    };
+    virtual void detach(JSGlobalObject*, ReasonForDetach);
 
     BreakpointID setBreakpoint(Breakpoint, unsigned& actualLine, unsigned& actualColumn);
     void removeBreakpoint(BreakpointID);

Modified: trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp (162734 => 162735)


--- trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/_javascript_Core/runtime/JSGlobalObject.cpp	2014-01-24 23:44:50 UTC (rev 162735)
@@ -164,7 +164,7 @@
 JSGlobalObject::~JSGlobalObject()
 {
     if (m_debugger)
-        m_debugger->detach(this);
+        m_debugger->detach(this, Debugger::GlobalObjectIsDestructing);
 
     if (LegacyProfiler* profiler = vm().enabledProfiler())
         profiler->stopProfiling(this);

Modified: trunk/Source/WebCore/ChangeLog (162734 => 162735)


--- trunk/Source/WebCore/ChangeLog	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/WebCore/ChangeLog	2014-01-24 23:44:50 UTC (rev 162735)
@@ -1,3 +1,18 @@
+2014-01-24  Mark Lam  <mark....@apple.com>
+
+        ASSERT(!m_markedSpace.m_currentDelayedReleaseScope) reloading page in inspector.
+        <https://webkit.org/b/127582>
+
+        Reviewed by Mark Hahnenberg.
+
+        No new tests.
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::attachDebugger):
+        * bindings/js/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::detachDebugger):
+        - Adding reasons for detaching a globalObject from the debugger.
+
 2014-01-24  Zalan Bujtas  <za...@apple.com>
 
         Subpixel rendering: Make PaintInfo layout unit aware.

Modified: trunk/Source/WebCore/bindings/js/ScriptController.cpp (162734 => 162735)


--- trunk/Source/WebCore/bindings/js/ScriptController.cpp	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/WebCore/bindings/js/ScriptController.cpp	2014-01-24 23:44:50 UTC (rev 162735)
@@ -286,7 +286,7 @@
     if (debugger)
         debugger->attach(globalObject);
     else if (JSC::Debugger* currentDebugger = globalObject->debugger())
-        currentDebugger->detach(globalObject);
+        currentDebugger->detach(globalObject, JSC::Debugger::TerminatingDebuggingSession);
 }
 
 void ScriptController::updateDocument()

Modified: trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp (162734 => 162735)


--- trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp	2014-01-24 23:39:45 UTC (rev 162734)
+++ trunk/Source/WebCore/bindings/js/WorkerScriptController.cpp	2014-01-24 23:44:50 UTC (rev 162735)
@@ -200,7 +200,7 @@
 
 void WorkerScriptController::detachDebugger(JSC::Debugger* debugger)
 {
-    debugger->detach(m_workerGlobalScopeWrapper->globalObject());
+    debugger->detach(m_workerGlobalScopeWrapper->globalObject(), JSC::Debugger::TerminatingDebuggingSession);
 }
 
 } // namespace WebCore
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to