[inspector] added timeout for Debugger.evaluateOnCallFrame method

R=dgozman@chromium.org,yangguo@chromium.org

Bug: none
Cq-Include-Trybots: luci.chromium.try:linux_chromium_rel_ng;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I569899f245190ca2fa720bdb837db1263e8058d5
Reviewed-on: https://chromium-review.googlesource.com/1023035
Commit-Queue: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52798}

src/debug/debug-stack-trace-iterator.cc

@@ -172,6 +172,8 @@ v8::MaybeLocal<v8::Value> DebugStackTraceIterator::Evaluate(
     v8::Local<v8::String> source, bool throw_on_side_effect) {
   DCHECK(!Done());
   Handle<Object> 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)

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",

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": [

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

src/inspector/v8-debugger-agent-impl.cc

@@ -1111,7 +1111,8 @@ Response V8DebuggerAgentImpl::evaluateOnCallFrame(
     const String16& callFrameId, const String16& expression,
     Maybe<String16> objectGroup, Maybe<bool> includeCommandLineAPI,
     Maybe<bool> silent, Maybe<bool> returnByValue, Maybe<bool> generatePreview,
-    Maybe<bool> throwOnSideEffect, std::unique_ptr<RemoteObject>* result,
+    Maybe<bool> throwOnSideEffect, Maybe<double> timeout,
+    std::unique_ptr<RemoteObject>* result,
     Maybe<protocol::Runtime::ExceptionDetails>* 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<v8::Value> maybeResultValue = it->Evaluate(
-      toV8String(m_isolate, expression), throwOnSideEffect.fromMaybe(false));
+
+  v8::MaybeLocal<v8::Value> 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();

src/inspector/v8-debugger-agent-impl.h

@@ -105,6 +105,7 @@ class V8DebuggerAgentImpl : public protocol::Debugger::Backend {
       Maybe<String16> objectGroup, Maybe<bool> includeCommandLineAPI,
       Maybe<bool> silent, Maybe<bool> returnByValue,
       Maybe<bool> generatePreview, Maybe<bool> throwOnSideEffect,
+      Maybe<double> timeout,
       std::unique_ptr<protocol::Runtime::RemoteObject>* result,
       Maybe<protocol::Runtime::ExceptionDetails>*) override;
   Response setVariableValue(

src/inspector/v8-inspector-impl.cc

@@ -32,6 +32,7 @@
 
 #include <vector>
 
+#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> 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;
+};
 
-void V8InspectorImpl::evaluateFinished(intptr_t id) {
-  m_runningEvaluates.erase(id);
+V8InspectorImpl::EvaluateScope::~EvaluateScope() {
+  if (m_cancelToken) {
+    v8::base::LockGuard<v8::base::Mutex> lock(&m_cancelToken->m_mutex);
+    m_cancelToken->m_canceled = true;
+    m_isolate->CancelTerminateExecution();
+  }
 }
 
-bool V8InspectorImpl::evaluateStillRunning(intptr_t id) {
-  return m_runningEvaluates.find(id) != m_runningEvaluates.end();
+class V8InspectorImpl::EvaluateScope::TerminateTask : public v8::Task {
+ public:
+  TerminateTask(v8::Isolate* isolate, std::shared_ptr<CancelToken> token)
+      : m_isolate(isolate), m_token(token) {}
+
+  void Run() {
+    // CancelToken contains m_canceled bool which may be changed from main
+    // thread, so lock mutex first.
+    v8::base::LockGuard<v8::base::Mutex> lock(&m_token->m_mutex);
+    if (m_token->m_canceled) return;
+    m_isolate->TerminateExecution();
+  }
+
+ private:
+  v8::Isolate* m_isolate;
+  std::shared_ptr<CancelToken> m_token;
+};
+
+protocol::Response V8InspectorImpl::EvaluateScope::setTimeout(double timeout) {
+  if (m_isolate->IsExecutionTerminating()) {
+    return protocol::Response::Error("Execution was terminated");
+  }
+  std::shared_ptr<v8::TaskRunner> 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<TerminateTask>(m_isolate, m_cancelToken), timeout);
+  return protocol::Response::OK();
 }
 
 }  // namespace v8_inspector

src/inspector/v8-inspector-impl.h

@@ -33,9 +33,9 @@
 
 #include <functional>
 #include <map>
-#include <set>
 
 #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<void(V8InspectorSessionImpl*)> 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<CancelToken> m_cancelToken;
+    v8::Isolate::SafeForTerminationScope m_safeForTerminationScope;
+  };
 
  private:
   v8::Isolate* m_isolate;
@@ -156,9 +167,6 @@ class V8InspectorImpl : public V8Inspector {
 
   std::unique_ptr<V8Console> m_console;
 
-  intptr_t m_lastEvaluateId = 0;
-  std::set<intptr_t> m_runningEvaluates;
-
   DISALLOW_COPY_AND_ASSIGN(V8InspectorImpl);
 };
 

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<void*>(m_evaluateId));
-  }
-
-  static void InterruptCallback(v8::Isolate* isolate, void* rawEvaluateId) {
-    intptr_t evaluateId = reinterpret_cast<intptr_t>(rawEvaluateId);
-    V8InspectorImpl* inspector =
-        static_cast<V8InspectorImpl*>(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<v8::TaskRunner> 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<TerminateTask>(m_inspector, evaluateId),
-        timeout.fromJust() / 1000.0);
-  }
   v8::MaybeLocal<v8::Value> 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.

test/inspector/debugger/evaluate-on-call-frame-timeout-expected.txt

+Tests that Debugger.evaluateOnCallFrame's timeout argument
+Run trivial expression:
+Evaluate finished!
+Run expression without interrupts:
+Evaluate finished!
+Run infinite loop:
+Evaluate finished!

test/inspector/debugger/evaluate-on-call-frame-timeout.js

+// 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();
+})();