Added VM support for isolate stacktrace status.

runtime/vm/isolate.cc

Index: runtime/vm/isolate.cc
===================================================================
--- runtime/vm/isolate.cc	(revision 20638)
+++ runtime/vm/isolate.cc	(working copy)
@@ -6,6 +6,7 @@
 
 #include "include/dart_api.h"
 #include "platform/assert.h"
+#include "platform/json.h"
 #include "lib/mirrors.h"
 #include "vm/code_observers.h"
 #include "vm/compiler_stats.h"
@@ -342,7 +343,8 @@
       deopt_fpu_registers_copy_(NULL),
       deopt_frame_copy_(NULL),
       deopt_frame_copy_size_(0),
-      deferred_objects_(NULL) {
+      deferred_objects_(NULL),
+      stacktrace_(NULL) {
 }
 
 
@@ -689,60 +691,115 @@
 }
 
 
+static Monitor* status_sync;
+
+bool Isolate::FetchStacktrace() {
+  Isolate* isolate = reinterpret_cast<Isolate*>(Dart_CurrentIsolate());
+  MonitorLocker ml(status_sync);
+  DebuggerStackTrace* stack = Debugger::CollectStackTrace();
+  TextBuffer buffer(256);
+  buffer.Printf("{ \"handle\": \"0x%"Px64"\", \"stacktrace\": [ ",
+                reinterpret_cast<int64_t>(isolate));
+  intptr_t n_frames = stack->Length();
+  for (int i = 0; i < n_frames; i++) {
+    if (i > 0) {
+      buffer.Printf(", ");
+    }
+    ActivationFrame* frame = stack->ActivationFrameAt(i);
+    const String& url = String::Handle(frame->SourceUrl());
+    const String& function =
+        String::Handle(frame->function().UserVisibleName());

[siva, 2013/03/28 22:09:00]

These handle creations can be hoisted outside the for loop that would ensure we
reuse handles and not create new ones each time.

[Tom Ball, 2013/03/29 22:39:32]

Good idea -- done.

+    buffer.Printf("{ \"url\": \"%s\", ", url.ToCString());
+    buffer.Printf("\"line\": %d, ", frame->LineNumber());
+    buffer.Printf("\"function\": \"%s\", ", function.ToCString());

[siva, 2013/03/28 22:09:00]

We probably also need the code object associated with the frame in case we want
to be able to click and get to the generated code for the function (this frame
could be running optimized code or unoptimized code).

[Tom Ball, 2013/03/29 22:39:32]

I added a "code": { "alive": bool, "optimized": bool } section. As we discussed
further code details would be returned in a separate, per-frame details query.

+    buffer.Printf("\"local_vars\": [ ");
+    intptr_t n_local_vars = frame->NumLocalVariables();

[siva, 2013/03/28 22:09:00]

Do we need a detailed local variable list for each frame to be collected here.
It could be another call when somebody clicks on the stack frame link....

[Tom Ball, 2013/03/29 22:39:32]

As we discussed, I removed this section in favor of a separate per-frame details
query.

+    for (int j = 0; j < n_local_vars; j++) {
+      if (j > 0) {
+        buffer.Printf(", ");
+      }
+      String& var_name = String::Handle();
+      Instance& value = Instance::Handle();

[siva, 2013/03/28 22:09:00]

These handle creations can be hoisted outside the for loop that would ensure we
reuse handles and not create new ones each time.

[Tom Ball, 2013/03/29 22:39:32]

Done.

+      intptr_t token_pos, end_pos;
+      frame->VariableAt(j, &var_name, &token_pos, &end_pos, &value);
+      buffer.Printf(
+          "{ \"name\": \"%s\", \"pos\": %d, \"end_pos\": %d, "
+          "\"value\": \"%s\" }",
+          var_name.ToCString(), token_pos, end_pos, value.ToCString());
+    }
+    buffer.Printf("]}");
+  }
+  buffer.Printf("]}");
+  isolate->stacktrace_ = strndup(buffer.buf(), buffer.length());

[siva, 2013/03/28 22:09:00]

Who is responsible for freeing stacktrace_ ?

[Tom Ball, 2013/03/29 22:39:32]

VmStats::WebService(), after the returned status is copied to the HTTP response.
 I added a comment to make that clear.

+  ml.Notify();
+  return true;
+}
+
+
+const char* Isolate::GetStatusStacktrace() {
+  Dart_IsolateInterruptCallback saved_callback = InterruptCallback();
+  SetInterruptCallback(&FetchStacktrace);
+  status_sync = new Monitor();

[siva, 2013/03/28 22:09:00]

Why do we allocate a new monitor for each stack trace request?

[Tom Ball, 2013/03/29 22:39:32]

We don't -- I was being overly conservative. Now the monitor is lazily created
once.

+  ScheduleInterrupts(Isolate::kApiInterrupt);

[siva, 2013/03/28 22:09:00]

Not sure if kApiInterrupt bit is the right one to use here.
Why could send an OOB message by posting the request string as the OOB message
or grab a new interrupt bit for VM stats and have it process the request string.

[Tom Ball, 2013/03/29 22:39:32]

I don't know, but copied what the debugger does -- perhaps both places need
improvement, so I welcome any specific suggestions.

+  {
+    MonitorLocker ml(status_sync);
+    ml.Wait();
+  }
+  SetInterruptCallback(saved_callback);
+  delete status_sync;
+  status_sync = NULL;
+  ASSERT(stacktrace_ != NULL);
+  const char* result = stacktrace_;
+  stacktrace_ = NULL;
+  return result;
+}
+
+
 char* Isolate::GetStatus(const char* request) {
   char* p = const_cast<char*>(request);
   const char* service_type = "/isolate/";
-  ASSERT(strncmp(p, service_type, strlen(service_type)) == 0);
+  ASSERT(!strncmp(p, service_type, strlen(service_type)));
   p += strlen(service_type);
 
   // Extract isolate handle.
   int64_t addr;
   OS::StringToInt64(p, &addr);
   Isolate* isolate = reinterpret_cast<Isolate*>(addr);
-  Heap* heap = isolate->heap();
+  p += strcspn(p, "/");
 
-  char buffer[256];
-  int64_t port = isolate->main_port();
-  int64_t start_time = (isolate->start_time() / 1000L);
-#if defined(TARGET_ARCH_X64)
-  const char* format = "{\n"
-      "  \"name\": \"%s\",\n"
-      "  \"port\": %ld,\n"
-      "  \"starttime\": %ld,\n"
-      "  \"stacklimit\": %ld,\n"
-      "  \"newspace\": {\n"
-      "    \"used\": %ld,\n"
-      "    \"capacity\": %ld\n"
-      "  },\n"
-      "  \"oldspace\": {\n"
-      "    \"used\": %ld,\n"
-      "    \"capacity\": %ld\n"
-      "  }\n"
-      "}";
-#else
-  const char* format = "{\n"
-      "  \"name\": \"%s\",\n"
-      "  \"port\": %lld,\n"
-      "  \"starttime\": %lld,\n"
-      "  \"stacklimit\": %d,\n"
-      "  \"newspace\": {\n"
-      "    \"used\": %d,\n"
-      "    \"capacity\": %d\n"
-      "  },\n"
-      "  \"oldspace\": {\n"
-      "    \"used\": %d,\n"
-      "    \"capacity\": %d\n"
-      "  }\n"
-      "}";
-#endif
-  int n = OS::SNPrint(buffer, 256, format, isolate->name(), port, start_time,
-                      isolate->saved_stack_limit(),
-                      heap->Used(Heap::kNew) / KB,
-                      heap->Capacity(Heap::kNew) / KB,
-                      heap->Used(Heap::kOld) / KB,
-                      heap->Capacity(Heap::kOld) / KB);
-  ASSERT(n < 256);
-  return strdup(buffer);
+  const char* trace_request = "/stacktrace";
+  if (!strncmp(p, trace_request, strlen(trace_request))) {

[siva, 2013/03/28 22:09:00]

It might be easier for adding new commands if we made this a table driven
dispatch...

[Tom Ball, 2013/03/29 22:39:32]

It's not quite a table, but GetStatus() now just parses the request and
dispatches to status functions.

+    return const_cast<char*>(isolate->GetStatusStacktrace());
+  } else {
+    const char* name = isolate->name();
+    int64_t port = isolate->main_port();
+    int64_t start_time = (isolate->start_time() / 1000L);
+    Heap* heap = isolate->heap();
+    const char* format = "{\n"
+        "  \"handle\": \"0x%"Px64"\",\n"
+        "  \"name\": \"%s\",\n"
+        "  \"port\": %"Pd",\n"
+        "  \"starttime\": %"Pd",\n"
+        "  \"stacklimit\": %"Pd",\n"
+        "  \"newspace\": {\n"
+        "    \"used\": %"Pd",\n"
+        "    \"capacity\": %"Pd"\n"
+        "  },\n"
+        "  \"oldspace\": {\n"
+        "    \"used\": %"Pd",\n"
+        "    \"capacity\": %"Pd"\n"
+        "  }\n"
+        "}";
+    char buffer[300];
+    int n = OS::SNPrint(buffer, 300, format, addr, name, port, start_time,
+                        isolate->saved_stack_limit(),
+                        heap->Used(Heap::kNew) / KB,
+                        heap->Capacity(Heap::kNew) / KB,
+                        heap->Used(Heap::kOld) / KB,
+                        heap->Capacity(Heap::kOld) / KB);
+    ASSERT(n < 300);
+    return strdup(buffer);
+  }
 }
 
 }  // namespace dart