[inspector] better stacks for promises

src/inspector/v8-debugger.cc

 // Copyright 2016 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.
 
 #include "src/inspector/v8-debugger.h"
 
 #include "src/inspector/debugger-script.h"
 #include "src/inspector/inspected-context.h"
 #include "src/inspector/protocol/Protocol.h"
 #include "src/inspector/script-breakpoint.h"
@@ skipping 895 matching lines @@
 void V8Debugger::asyncTaskCanceledForStack(void* task) {
   if (!m_maxAsyncCallStackDepth) return;
   m_asyncTaskStacks.erase(task);
   m_recurringTasks.erase(task);
   m_parentTask.erase(task);
   m_asyncTaskCreationStacks.erase(task);
 }
 
 void V8Debugger::asyncTaskStartedForStack(void* task) {
   if (!m_maxAsyncCallStackDepth) return;
-  m_currentTasks.push_back(task);
-  auto parentIt = m_parentTask.find(task);
-  AsyncTaskToStackTrace::iterator stackIt = m_asyncTaskStacks.find(
-      parentIt == m_parentTask.end() ? task : parentIt->second);
   // Needs to support following order of events:
   // - asyncTaskScheduled
   //   <-- attached here -->
   // - asyncTaskStarted
   // - asyncTaskCanceled <-- canceled before finished
   //   <-- async stack requested here -->
   // - asyncTaskFinished
-  std::weak_ptr<AsyncStackTrace> asyncParent;
-  if (stackIt != m_asyncTaskStacks.end()) asyncParent = stackIt->second;
+  m_currentTasks.push_back(task);
+  auto parentIt = m_parentTask.find(task);
+  AsyncTaskToStackTrace::iterator stackIt = m_asyncTaskStacks.find(
+      parentIt == m_parentTask.end() ? task : parentIt->second);
+  if (stackIt != m_asyncTaskStacks.end()) {
+    m_currentAsyncParent.push_back(stackIt->second.lock());
+  } else {
+    m_currentAsyncParent.emplace_back();
+  }
   auto itCreation = m_asyncTaskCreationStacks.find(task);
-  if (asyncParent.lock() && itCreation != m_asyncTaskCreationStacks.end()) {

[kozy, 2017/04/27 18:35:31]

this change is essential of this CL.

+  if (itCreation != m_asyncTaskCreationStacks.end()) {
     m_currentAsyncCreation.push_back(itCreation->second.lock());
   } else {
     m_currentAsyncCreation.emplace_back();
   }
-  m_currentAsyncParent.push_back(asyncParent.lock());
 }
 
 void V8Debugger::asyncTaskFinishedForStack(void* task) {
   if (!m_maxAsyncCallStackDepth) return;
   // We could start instrumenting half way and the stack is empty.
   if (!m_currentTasks.size()) return;
   DCHECK(m_currentTasks.back() == task);
   m_currentTasks.pop_back();
 
   DCHECK(m_currentAsyncParent.size() == m_currentAsyncCreation.size());
@@ skipping 86 matching lines @@
   }
   cleanupExpiredWeakPointers(m_asyncTaskStacks);
   cleanupExpiredWeakPointers(m_asyncTaskCreationStacks);
   for (auto it = m_recurringTasks.begin(); it != m_recurringTasks.end();) {
     if (m_asyncTaskStacks.find(*it) == m_asyncTaskStacks.end()) {
       it = m_recurringTasks.erase(it);
     } else {
       ++it;
     }
   }
+  // We use parent information to provide correct parent stack for promises,
+  // Promise.resolve().then(...).then(...)
+  // created task#1 - stack#1
+  // scheduled task#1 - stack#2
+  // created task#2 -> task#1 - stack#3
+  // created task#3 -> task#2 - stack#4
+  // ...
+  // started task#2 - use stack(parent(task#2)) == stack(task#1) == stack#2
+  // ...
+  // scheduled task#2 - stack#5

[dgozman, 2017/04/27 17:26:34]

scheduled after started?

[kozy, 2017/04/27 18:35:31]

yes, it's how our promise instrumentation works and why on started we get
scheduled stack from parent.

+  // finished task#2
+  // started task#3 - use stack(parent(task#3)) == stack(task#2) == stack#5
+  // ...
+  // scheduled task#3 - stack#6
+  // finished task#3
+  // Let's consider what parent link is obsolete when we collect stacks:
+  // stack#1 - (task#2 -> task#1, task#3 -> task#2)
+  // stack#2 - (task#3 -> task#2)
+  // stack#3 - (task#3 -> task#2)
+  // stack#4 - (task#3 -> task#2)
+  // stack#5 - ()
+  // stack#6 - ()
+  // But in case when we start collecting stacks after getting stack#4 but
+  // before stack#5 is added we should preserve parent link since original task
+  // can be scheduled at any time in future and this link will be required to
+  // get correct parent stack later. It means that we should collect parent link
+  // in case when there is no creation or schedule stack for parent.
   for (auto it = m_parentTask.begin(); it != m_parentTask.end();) {
     if (m_asyncTaskCreationStacks.find(it->second) ==
-        m_asyncTaskCreationStacks.end()) {
+            m_asyncTaskCreationStacks.end() &&
+        m_asyncTaskStacks.find(it->second) == m_asyncTaskStacks.end()) {
       it = m_parentTask.erase(it);
     } else {
       ++it;
     }
   }
   cleanupExpiredWeakPointers(m_framesCache);
 }
 
 std::shared_ptr<StackFrame> V8Debugger::symbolize(
     v8::Local<v8::StackFrame> v8Frame) {
@@ skipping 24 matching lines @@
   fprintf(stdout, "Async stacks count: %d\n", m_asyncStacksCount);
   fprintf(stdout, "Scheduled async tasks: %zu\n", m_asyncTaskStacks.size());
   fprintf(stdout, "Created async tasks: %zu\n",
           m_asyncTaskCreationStacks.size());
   fprintf(stdout, "Async tasks with parent: %zu\n", m_parentTask.size());
   fprintf(stdout, "Recurring async tasks: %zu\n", m_recurringTasks.size());
   fprintf(stdout, "\n");
 }
 
 }  // namespace v8_inspector