diff --git a/src/debug/debug-stack-trace-iterator.cc b/src/debug/debug-stack-trace-iterator.cc index 0880f2b80f..ab703f1f0e 100644 --- a/src/debug/debug-stack-trace-iterator.cc +++ b/src/debug/debug-stack-trace-iterator.cc @@ -172,6 +172,8 @@ v8::MaybeLocal DebugStackTraceIterator::Evaluate( v8::Local source, bool throw_on_side_effect) { DCHECK(!Done()); Handle value; + i::SafeForInterruptsScope safe_for_interrupt_scope( + isolate_, i::StackGuard::TERMINATE_EXECUTION); if (!DebugEvaluate::Local(isolate_, iterator_.frame()->id(), inlined_frame_index_, Utils::OpenHandle(*source), throw_on_side_effect) diff --git a/src/inspector/DEPS b/src/inspector/DEPS index f396d64b99..e3457ca0f9 100644 --- a/src/inspector/DEPS +++ b/src/inspector/DEPS @@ -5,6 +5,7 @@ include_rules = [ "+src/base/macros.h", "+src/base/logging.h", "+src/base/platform/platform.h", + "+src/base/platform/mutex.h", "+src/conversions.h", "+src/flags.h", "+src/utils.h", diff --git a/src/inspector/js_protocol.json b/src/inspector/js_protocol.json index ca1daaec8d..dc473849ee 100644 --- a/src/inspector/js_protocol.json +++ b/src/inspector/js_protocol.json @@ -391,6 +391,13 @@ "description": "Whether to throw an exception if side effect cannot be ruled out during evaluation.", "optional": true, "type": "boolean" + }, + { + "name": "timeout", + "description": "Terminate execution after timing out (number of milliseconds).", + "experimental": true, + "optional": true, + "$ref": "Runtime.TimeDelta" } ], "returns": [ diff --git a/src/inspector/js_protocol.pdl b/src/inspector/js_protocol.pdl index 0253e10dd3..9f02e9d35c 100644 --- a/src/inspector/js_protocol.pdl +++ b/src/inspector/js_protocol.pdl @@ -191,6 +191,8 @@ domain Debugger experimental optional boolean generatePreview # Whether to throw an exception if side effect cannot be ruled out during evaluation. optional boolean throwOnSideEffect + # Terminate execution after timing out (number of milliseconds). + experimental optional Runtime.TimeDelta timeout returns # Object wrapper for the evaluation result. Runtime.RemoteObject result diff --git a/src/inspector/v8-debugger-agent-impl.cc b/src/inspector/v8-debugger-agent-impl.cc index 3303257010..9216ab2f84 100644 --- a/src/inspector/v8-debugger-agent-impl.cc +++ b/src/inspector/v8-debugger-agent-impl.cc @@ -1111,7 +1111,8 @@ Response V8DebuggerAgentImpl::evaluateOnCallFrame( const String16& callFrameId, const String16& expression, Maybe objectGroup, Maybe includeCommandLineAPI, Maybe silent, Maybe returnByValue, Maybe generatePreview, - Maybe throwOnSideEffect, std::unique_ptr* result, + Maybe throwOnSideEffect, Maybe timeout, + std::unique_ptr* result, Maybe* exceptionDetails) { if (!isPaused()) return Response::Error(kDebuggerNotPaused); InjectedScript::CallFrameScope scope(m_session, callFrameId); @@ -1125,8 +1126,17 @@ Response V8DebuggerAgentImpl::evaluateOnCallFrame( if (it->Done()) { return Response::Error("Could not find call frame with given id"); } - v8::MaybeLocal maybeResultValue = it->Evaluate( - toV8String(m_isolate, expression), throwOnSideEffect.fromMaybe(false)); + + v8::MaybeLocal maybeResultValue; + { + V8InspectorImpl::EvaluateScope evaluateScope(m_isolate); + if (timeout.isJust()) { + response = evaluateScope.setTimeout(timeout.fromJust() / 1000.0); + if (!response.isSuccess()) return response; + } + maybeResultValue = it->Evaluate(toV8String(m_isolate, expression), + throwOnSideEffect.fromMaybe(false)); + } // Re-initialize after running client's code, as it could have destroyed // context or session. response = scope.initialize(); diff --git a/src/inspector/v8-debugger-agent-impl.h b/src/inspector/v8-debugger-agent-impl.h index ec823853a3..f5623a8dc5 100644 --- a/src/inspector/v8-debugger-agent-impl.h +++ b/src/inspector/v8-debugger-agent-impl.h @@ -105,6 +105,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend { Maybe objectGroup, Maybe includeCommandLineAPI, Maybe silent, Maybe returnByValue, Maybe generatePreview, Maybe throwOnSideEffect, + Maybe timeout, std::unique_ptr* result, Maybe*) override; Response setVariableValue( diff --git a/src/inspector/v8-inspector-impl.cc b/src/inspector/v8-inspector-impl.cc index 64690d6ac0..7fe5e383bd 100644 --- a/src/inspector/v8-inspector-impl.cc +++ b/src/inspector/v8-inspector-impl.cc @@ -32,6 +32,7 @@ #include +#include "src/base/platform/mutex.h" #include "src/inspector/inspected-context.h" #include "src/inspector/string-util.h" #include "src/inspector/v8-console-agent-impl.h" @@ -44,6 +45,8 @@ #include "src/inspector/v8-runtime-agent-impl.h" #include "src/inspector/v8-stack-trace-impl.h" +#include "include/v8-platform.h" + namespace v8_inspector { std::unique_ptr V8Inspector::create(v8::Isolate* isolate, @@ -376,18 +379,53 @@ void V8InspectorImpl::forEachSession( } } -intptr_t V8InspectorImpl::evaluateStarted() { - intptr_t id = ++m_lastEvaluateId; - m_runningEvaluates.insert(id); - return id; +V8InspectorImpl::EvaluateScope::EvaluateScope(v8::Isolate* isolate) + : m_isolate(isolate), m_safeForTerminationScope(isolate) {} + +struct V8InspectorImpl::EvaluateScope::CancelToken { + v8::base::Mutex m_mutex; + bool m_canceled = false; +}; + +V8InspectorImpl::EvaluateScope::~EvaluateScope() { + if (m_cancelToken) { + v8::base::LockGuard lock(&m_cancelToken->m_mutex); + m_cancelToken->m_canceled = true; + m_isolate->CancelTerminateExecution(); + } } -void V8InspectorImpl::evaluateFinished(intptr_t id) { - m_runningEvaluates.erase(id); -} +class V8InspectorImpl::EvaluateScope::TerminateTask : public v8::Task { + public: + TerminateTask(v8::Isolate* isolate, std::shared_ptr token) + : m_isolate(isolate), m_token(token) {} -bool V8InspectorImpl::evaluateStillRunning(intptr_t id) { - return m_runningEvaluates.find(id) != m_runningEvaluates.end(); + void Run() { + // CancelToken contains m_canceled bool which may be changed from main + // thread, so lock mutex first. + v8::base::LockGuard lock(&m_token->m_mutex); + if (m_token->m_canceled) return; + m_isolate->TerminateExecution(); + } + + private: + v8::Isolate* m_isolate; + std::shared_ptr m_token; +}; + +protocol::Response V8InspectorImpl::EvaluateScope::setTimeout(double timeout) { + if (m_isolate->IsExecutionTerminating()) { + return protocol::Response::Error("Execution was terminated"); + } + std::shared_ptr taskRunner = + v8::debug::GetCurrentPlatform()->GetWorkerThreadsTaskRunner(m_isolate); + if (!taskRunner) { + return protocol::Response::Error("Timeout is not supported by embedder"); + } + m_cancelToken.reset(new CancelToken()); + taskRunner->PostDelayedTask( + v8::base::make_unique(m_isolate, m_cancelToken), timeout); + return protocol::Response::OK(); } } // namespace v8_inspector diff --git a/src/inspector/v8-inspector-impl.h b/src/inspector/v8-inspector-impl.h index cb5154f410..73a9253e9e 100644 --- a/src/inspector/v8-inspector-impl.h +++ b/src/inspector/v8-inspector-impl.h @@ -33,9 +33,9 @@ #include #include -#include #include "src/base/macros.h" +#include "src/base/platform/mutex.h" #include "src/inspector/protocol/Protocol.h" #include "include/v8-inspector.h" @@ -121,9 +121,20 @@ class V8InspectorImpl : public V8Inspector { void forEachSession(int contextGroupId, std::function callback); - intptr_t evaluateStarted(); - void evaluateFinished(intptr_t); - bool evaluateStillRunning(intptr_t); + class EvaluateScope { + public: + explicit EvaluateScope(v8::Isolate* isolate); + ~EvaluateScope(); + + protocol::Response setTimeout(double timeout); + + private: + v8::Isolate* m_isolate; + class TerminateTask; + struct CancelToken; + std::shared_ptr m_cancelToken; + v8::Isolate::SafeForTerminationScope m_safeForTerminationScope; + }; private: v8::Isolate* m_isolate; @@ -156,9 +167,6 @@ class V8InspectorImpl : public V8Inspector { std::unique_ptr m_console; - intptr_t m_lastEvaluateId = 0; - std::set m_runningEvaluates; - DISALLOW_COPY_AND_ASSIGN(V8InspectorImpl); }; diff --git a/src/inspector/v8-runtime-agent-impl.cc b/src/inspector/v8-runtime-agent-impl.cc index 3fbae08023..5e77868cd3 100644 --- a/src/inspector/v8-runtime-agent-impl.cc +++ b/src/inspector/v8-runtime-agent-impl.cc @@ -214,31 +214,6 @@ Response ensureContext(V8InspectorImpl* inspector, int contextGroupId, return Response::OK(); } -class TerminateTask : public v8::Task { - public: - TerminateTask(V8InspectorImpl* inspector, intptr_t evaluateId) - : m_inspector(inspector), m_evaluateId(evaluateId) {} - - void Run() { - if (!m_inspector->evaluateStillRunning(m_evaluateId)) return; - // We should call terminateExecution on the main thread. - m_inspector->isolate()->RequestInterrupt( - InterruptCallback, reinterpret_cast(m_evaluateId)); - } - - static void InterruptCallback(v8::Isolate* isolate, void* rawEvaluateId) { - intptr_t evaluateId = reinterpret_cast(rawEvaluateId); - V8InspectorImpl* inspector = - static_cast(v8::debug::GetInspector(isolate)); - if (!inspector->evaluateStillRunning(evaluateId)) return; - inspector->debugger()->terminateExecution(nullptr); - } - - private: - V8InspectorImpl* m_inspector; - intptr_t m_evaluateId; -}; - } // namespace V8RuntimeAgentImpl::V8RuntimeAgentImpl( @@ -283,31 +258,22 @@ void V8RuntimeAgentImpl::evaluate( // Temporarily enable allow evals for inspector. scope.allowCodeGenerationFromStrings(); - - V8InspectorImpl* inspector = m_inspector; - intptr_t evaluateId = inspector->evaluateStarted(); - if (timeout.isJust()) { - std::shared_ptr taskRunner = - v8::debug::GetCurrentPlatform()->GetWorkerThreadsTaskRunner( - m_inspector->isolate()); - if (!taskRunner) { - callback->sendFailure( - Response::Error("Timeout is not supported by embedder")); - return; - } - taskRunner->PostDelayedTask( - v8::base::make_unique(m_inspector, evaluateId), - timeout.fromJust() / 1000.0); - } v8::MaybeLocal maybeResultValue; { + V8InspectorImpl::EvaluateScope evaluateScope(m_inspector->isolate()); + if (timeout.isJust()) { + response = evaluateScope.setTimeout(timeout.fromJust() / 1000.0); + if (!response.isSuccess()) { + callback->sendFailure(response); + return; + } + } v8::MicrotasksScope microtasksScope(m_inspector->isolate(), v8::MicrotasksScope::kRunMicrotasks); maybeResultValue = v8::debug::EvaluateGlobal( m_inspector->isolate(), toV8String(m_inspector->isolate(), expression), throwOnSideEffect.fromMaybe(false)); } // Run microtasks before returning result. - inspector->evaluateFinished(evaluateId); // Re-initialize after running client's code, as it could have destroyed // context or session. diff --git a/test/inspector/debugger/evaluate-on-call-frame-timeout-expected.txt b/test/inspector/debugger/evaluate-on-call-frame-timeout-expected.txt new file mode 100644 index 0000000000..1233761449 --- /dev/null +++ b/test/inspector/debugger/evaluate-on-call-frame-timeout-expected.txt @@ -0,0 +1,7 @@ +Tests that Debugger.evaluateOnCallFrame's timeout argument +Run trivial expression: +Evaluate finished! +Run expression without interrupts: +Evaluate finished! +Run infinite loop: +Evaluate finished! diff --git a/test/inspector/debugger/evaluate-on-call-frame-timeout.js b/test/inspector/debugger/evaluate-on-call-frame-timeout.js new file mode 100644 index 0000000000..02796f6c06 --- /dev/null +++ b/test/inspector/debugger/evaluate-on-call-frame-timeout.js @@ -0,0 +1,40 @@ +// Copyright 2018 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. + +const {Protocol} = InspectorTest.start( + `Tests that Debugger.evaluateOnCallFrame's timeout argument`); + +(async function test(){ + Protocol.Debugger.enable(); + Protocol.Runtime.evaluate({expression: "debugger;"}); + const {params:{callFrames:[{callFrameId}]}} = await Protocol.Debugger.oncePaused(); + { + InspectorTest.log('Run trivial expression:'); + const result = await Protocol.Debugger.evaluateOnCallFrame({ + callFrameId, + expression: 'function foo() {} foo()', + timeout: 0 + }); + InspectorTest.log('Evaluate finished!'); + } + { + InspectorTest.log('Run expression without interrupts:'); + const result = await Protocol.Debugger.evaluateOnCallFrame({ + callFrameId, + expression: '', + timeout: 0 + }); + InspectorTest.log('Evaluate finished!'); + } + { + InspectorTest.log('Run infinite loop:'); + const result = await Protocol.Debugger.evaluateOnCallFrame({ + callFrameId, + expression: 'for(;;){}', + timeout: 0 + }); + InspectorTest.log('Evaluate finished!'); + } + InspectorTest.completeTest(); +})();