From 32f433c12e5af0bc2c23d8cc9cdefaff990dc469 Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 26 May 2014 07:05:56 +0000 Subject: [PATCH] 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 --- src/debug-debugger.js | 12 +++++++--- src/debug.cc | 26 ++++++---------------- test/mjsunit/debug-scripts-request.js | 2 ++ test/mjsunit/regress/regress-debug-context-load.js | 8 +++++++ tools/generate-runtime-tests.py | 2 +- 5 files changed, 27 insertions(+), 23 deletions(-) create mode 100644 test/mjsunit/regress/regress-debug-context-load.js diff --git a/src/debug-debugger.js b/src/debug-debugger.js index 660ea79..5462b53 100644 --- 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 8956e92..900821b 100644 --- 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 e027563..f9fdde6 100644 --- 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 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 0bbac02..c5bbeae 100755 --- 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. -- 2.7.4