Fix leak in debug mirror cache.

When fetching loaded scripts, mirror objects are created and cached.
If the cache is not cleared, it holds script objects alive.

This also fixes a minor issue with script unloading.

R=ulan@chromium.org
BUG=376534
LOG=N

Review URL: https://codereview.chromium.org/296953005

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@21477 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/debug-debugger.js b/src/debug-debugger.js
index 660ea790389ae4e828681fe786683ee0f3a7a3a4..5462b53b6b0ba823218e144663bebcebff06facd 100644 (file)
--- a/src/debug-debugger.js
+++ b/src/debug-debugger.js
@@ -486,6 +486,12 @@ function GetScriptBreakPoints(script) {
 }
 
 
+function GetLoadedScripts() {
+  ClearMirrorCache();  // The mirror cache may be holding onto scripts.
+  return %DebugGetLoadedScripts();
+}
+
+
 Debug.setListener = function(listener, opt_data) {
   if (!IS_FUNCTION(listener) && !IS_UNDEFINED(listener) && !IS_NULL(listener)) {
     throw new Error('Parameters have wrong types.');
@@ -915,7 +921,7 @@ Debug.showBreakPoints = function(f, full, opt_position_alignment) {
 // scanning the heap.
 Debug.scripts = function() {
   // Collect all scripts in the heap.
-  return %DebugGetLoadedScripts();
+  return GetLoadedScripts();
 };
 
 
@@ -2244,7 +2250,7 @@ DebugCommandProcessor.prototype.scriptsRequest_ = function(request, response) {
   }
 
   // Collect all scripts in the heap.
-  var scripts = %DebugGetLoadedScripts();
+  var scripts = GetLoadedScripts();
 
   response.body = [];
 
@@ -2316,7 +2322,7 @@ DebugCommandProcessor.prototype.changeLiveRequest_ = function(
   var script_id = request.arguments.script_id;
   var preview_only = !!request.arguments.preview_only;
 
-  var scripts = %DebugGetLoadedScripts();
+  var scripts = GetLoadedScripts();
 
   var the_script = null;
   for (var i = 0; i < scripts.length; i++) {

diff --git a/src/debug.cc b/src/debug.cc
index 8956e92c7d9628343dedd1721a637de53cb8719e..900821beaf0471fee3f0f5bb68ba9dfcb866e4ea 100644 (file)
--- a/src/debug.cc
+++ b/src/debug.cc
@@ -3139,22 +3139,23 @@ void Debugger::SetMessageHandler(v8::Debug::MessageHandler handler) {
 
 
 void Debugger::UpdateState() {
+  Debug* debug = isolate_->debug();
   bool activate = message_handler_ != NULL ||
                   !event_listener_.is_null() ||
-                  isolate_->debug()->InDebugger();
+                  debug->InDebugger();
   if (!is_active_ && activate) {
     // Note that the debug context could have already been loaded to
     // bootstrap test cases.
     isolate_->compilation_cache()->Disable();
-    activate = isolate_->debug()->Load();
-  } else if (is_active_ && !activate) {
+    activate = debug->Load();
+  } else if (debug->IsLoaded() && !activate) {
     isolate_->compilation_cache()->Enable();
-    isolate_->debug()->ClearAllBreakPoints();
-    isolate_->debug()->Unload();
+    debug->ClearAllBreakPoints();
+    debug->Unload();
   }
   is_active_ = activate;
   // At this point the debug context is loaded iff the debugger is active.
-  ASSERT(isolate_->debug()->IsLoaded() == is_active_);
+  ASSERT(debug->IsLoaded() == is_active_);
 }
 
 
@@ -3271,22 +3272,9 @@ EnterDebugger::~EnterDebugger() {
     // JavaScript. This can happen if the v8::Debug::Call is used in which
     // case the exception should end up in the calling code.
     if (!isolate_->has_pending_exception()) {
-      // Try to avoid any pending debug break breaking in the clear mirror
-      // cache JavaScript code.
-      if (isolate_->stack_guard()->CheckDebugBreak()) {
-        debug->set_has_pending_interrupt(true);
-        isolate_->stack_guard()->ClearDebugBreak();
-      }
       debug->ClearMirrorCache();
     }
 
-    // Request debug break when leaving the last debugger entry
-    // if one was recorded while debugging.
-    if (debug->has_pending_interrupt()) {
-      debug->set_has_pending_interrupt(false);
-      isolate_->stack_guard()->RequestDebugBreak();
-    }
-
     // If there are commands in the queue when leaving the debugger request
     // that these commands are processed.
     if (isolate_->debugger()->HasCommands()) {

diff --git a/test/mjsunit/debug-scripts-request.js b/test/mjsunit/debug-scripts-request.js
index e027563b9b89d8eb714b7b73ec89b7729c5424af..f9fdde6348e53a3648295ab7ea029f2e70e1151c 100644 (file)
--- a/test/mjsunit/debug-scripts-request.js
+++ b/test/mjsunit/debug-scripts-request.js
@@ -108,3 +108,5 @@ debugger;
 assertTrue(listenerComplete,
            "listener did not run to completion, exception: " + exception);
 assertFalse(exception, "exception in listener")
+
+Debug.setListener(null);

diff --git a/test/mjsunit/regress/regress-debug-context-load.js b/test/mjsunit/regress/regress-debug-context-load.js
new file mode 100644 (file)
index 0000000..0b3c275
--- /dev/null
+++ b/test/mjsunit/regress/regress-debug-context-load.js
@@ -0,0 +1,8 @@
+// Copyright 2014 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Flags: --expose-debug-as debug
+
+Debug = debug.Debug;
+Debug.setListener(null);

diff --git a/tools/generate-runtime-tests.py b/tools/generate-runtime-tests.py
index 0bbac02026dcb72d5de3f7123826796be5be4c53..c5bbeaea4b86b0cc5e06ce3aefd042edf69f31c3 100755 (executable)
--- a/tools/generate-runtime-tests.py
+++ b/tools/generate-runtime-tests.py
@@ -51,7 +51,7 @@ EXPECTED_FUNCTION_COUNT = 358
 EXPECTED_FUZZABLE_COUNT = 325
 EXPECTED_CCTEST_COUNT = 6
 EXPECTED_UNKNOWN_COUNT = 5
-EXPECTED_BUILTINS_COUNT = 781
+EXPECTED_BUILTINS_COUNT = 782
 
 
 # Don't call these at all.