ASSERT(!m_markedSpace.m_currentDelayedReleaseScope) reloading page in inspector.
authormark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jan 2014 23:44:50 +0000 (23:44 +0000)
committermark.lam@apple.com <mark.lam@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Jan 2014 23:44:50 +0000 (23:44 +0000)
<https://webkit.org/b/127582>

Reviewed by Mark Hahnenberg.

Source/JavaScriptCore:

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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@162735 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/debugger/Debugger.cpp
Source/JavaScriptCore/debugger/Debugger.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/ScriptController.cpp
Source/WebCore/bindings/js/WorkerScriptController.cpp

index 7330ec6..e97417b 100644 (file)
@@ -1,3 +1,35 @@
+2014-01-24  Mark Lam  <mark.lam@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  <bfulgham@apple.com>
 
         [Win] Convert some NMake files to MSBuild project files
index d3f0734..b27c9dd 100644 (file)
@@ -176,7 +176,7 @@ void Debugger::attach(JSGlobalObject* globalObject)
     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 @@ void Debugger::detach(JSGlobalObject* globalObject)
     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 @@ void Debugger::setSteppingMode(SteppingMode mode)
 
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     SetSteppingModeFunctor functor(this, mode);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -313,7 +317,6 @@ void Debugger::toggleBreakpoint(Breakpoint& breakpoint, Debugger::BreakpointStat
 {
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     ToggleBreakpointFunctor functor(this, breakpoint, enabledOrNot);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -493,7 +496,6 @@ void Debugger::clearBreakpoints()
 
     if (!m_vm)
         return;
-    HeapIterationScope iterationScope(m_vm->heap);
     ClearCodeBlockDebuggerRequestsFunctor functor(this);
     m_vm->heap.forEachCodeBlock(functor);
 }
@@ -519,7 +521,6 @@ private:
 void Debugger::clearDebuggerRequests(JSGlobalObject* globalObject)
 {
     ASSERT(m_vm);
-    HeapIterationScope iterationScope(m_vm->heap);
     ClearDebuggerRequestsFunctor functor(globalObject);
     m_vm->heap.forEachCodeBlock(functor);
 }
index 8b1a08b..042ad8b 100644 (file)
@@ -61,7 +61,11 @@ public:
     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);
index b5d0970..2c274c1 100644 (file)
@@ -164,7 +164,7 @@ JSGlobalObject::JSGlobalObject(VM& vm, Structure* structure, const GlobalObjectM
 JSGlobalObject::~JSGlobalObject()
 {
     if (m_debugger)
-        m_debugger->detach(this);
+        m_debugger->detach(this, Debugger::GlobalObjectIsDestructing);
 
     if (LegacyProfiler* profiler = vm().enabledProfiler())
         profiler->stopProfiling(this);
index b6f6682..e8bb946 100644 (file)
@@ -1,3 +1,18 @@
+2014-01-24  Mark Lam  <mark.lam@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  <zalan@apple.com>
 
         Subpixel rendering: Make PaintInfo layout unit aware.
index 718f888..c0135ae 100644 (file)
@@ -286,7 +286,7 @@ void ScriptController::attachDebugger(JSDOMWindowShell* shell, JSC::Debugger* de
     if (debugger)
         debugger->attach(globalObject);
     else if (JSC::Debugger* currentDebugger = globalObject->debugger())
-        currentDebugger->detach(globalObject);
+        currentDebugger->detach(globalObject, JSC::Debugger::TerminatingDebuggingSession);
 }
 
 void ScriptController::updateDocument()
index 5de604c..579a0b4 100644 (file)
@@ -200,7 +200,7 @@ void WorkerScriptController::attachDebugger(JSC::Debugger* debugger)
 
 void WorkerScriptController::detachDebugger(JSC::Debugger* debugger)
 {
-    debugger->detach(m_workerGlobalScopeWrapper->globalObject());
+    debugger->detach(m_workerGlobalScopeWrapper->globalObject(), JSC::Debugger::TerminatingDebuggingSession);
 }
 
 } // namespace WebCore