Bug 1200446 - Add a method for saving heap snapshots to MemoryActor; r=jryans

testing/xpcshell/head.js

@@ -63,7 +63,7 @@ try {
   runningInParent = Components.classes["@mozilla.org/xre/runtime;1"].
                     getService(Components.interfaces.nsIXULRuntime).processType
                     == Components.interfaces.nsIXULRuntime.PROCESS_TYPE_DEFAULT;
-} 
+}
 catch (e) { }
 
 // Only if building of places is enabled.
@@ -127,7 +127,8 @@ try {
       return this;
     },
     observe : function (msg) {
-      do_print("CONSOLE_MESSAGE: (" + levelNames[msg.logLevel] + ") " + msg.toString());
+      if (typeof do_print === "function")
+        do_print("CONSOLE_MESSAGE: (" + levelNames[msg.logLevel] + ") " + msg.toString());
     }
   };
   Components.classes["@mozilla.org/consoleservice;1"]
@@ -504,7 +505,7 @@ function _execute_test() {
     do_test_pending("MAIN run_test");
     // Check if run_test() is defined. If defined, run it.
     // Else, call run_next_test() directly to invoke tests
-    // added by add_test() and add_task().  
+    // added by add_test() and add_task().
     if (typeof run_test === "function") {
       run_test();
     } else {
@@ -1433,7 +1434,7 @@ function run_next_test()
                     "run_next_test() should not be called from inside add_task() " +
                     "under any circumstances!");
   }
- 
+
   function _run_next_test()
   {
     if (_gTestIndex < _gTests.length) {

toolkit/devtools/DevToolsUtils.js

@@ -10,6 +10,9 @@ var { Ci, Cu, Cc, components } = require("chrome");
 var Services = require("Services");
 var promise = require("promise");
 
+loader.lazyRequireGetter(this, "FileUtils",
+                         "resource://gre/modules/FileUtils.jsm", true);
+
 /**
  * Turn the error |aError| into a string, without fail.
  */
@@ -713,3 +716,27 @@ Object.defineProperty(exports, "testing", {
     testing = state;
   }
 });
+
+/**
+ * Open the file at the given path for reading.
+ *
+ * @param {String} filePath
+ *
+ * @returns Promise<nsIInputStream>
+ */
+exports.openFileStream = function (filePath) {
+  return new Promise((resolve, reject) => {
+    const uri = NetUtil.newURI(new FileUtils.File(filePath));
+    NetUtil.asyncFetch(
+      { uri, loadUsingSystemPrincipal: true },
+      (stream, result) => {
+        if (!components.isSuccessCode(result)) {
+          reject(new Error(`Could not open "${filePath}": result = ${result}`));
+          return;
+        }
+
+        resolve(stream);
+      }
+    );
+  });
+}

toolkit/devtools/Loader.jsm

@@ -30,7 +30,8 @@ this.EXPORTED_SYMBOLS = ["DevToolsLoader", "devtools", "BuiltinProvider",
 let loaderModules = {
   "Services": Object.create(Services),
   "toolkit/loader": Loader,
-  "PromiseDebugging": PromiseDebugging
+  PromiseDebugging,
+  ThreadSafeChromeUtils,
 };
 XPCOMUtils.defineLazyGetter(loaderModules, "Debugger", () => {
   // addDebuggerToGlobal only allows adding the Debugger object to a global. The

toolkit/devtools/heapsnapshot/HeapSnapshotFileUtils.js

@@ -0,0 +1,95 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// Heap snapshots are always saved in the temp directory, and have a regular
+// naming convention. This module provides helpers for working with heap
+// snapshot files in a safe manner. Because we attempt to avoid unnecessary
+// copies of the heap snapshot files by checking the local filesystem for a heap
+// snapshot file with the given snapshot id, we want to ensure that we are only
+// attempting to open heap snapshot files and not `~/.ssh/id_rsa`, for
+// example. Therefore, the RDP only talks about snapshot ids, or transfering the
+// bulk file data. A file path can be recovered from a snapshot id, which allows
+// one to check for the presence of the heap snapshot file on the local file
+// system, but we don't have to worry about opening arbitrary files.
+//
+// The heap snapshot file path conventions permits the following forms:
+//
+//     $TEMP_DIRECTORY/XXXXXXXXXX.fxsnapshot
+//     $TEMP_DIRECTORY/XXXXXXXXXX-XXXXX.fxsnapshot
+//
+// Where the strings of "X" are zero or more digits.
+
+"use strict";
+
+const { Ci } = require("chrome");
+loader.lazyRequireGetter(this, "FileUtils",
+                         "resource://gre/modules/FileUtils.jsm", true);
+loader.lazyRequireGetter(this, "OS", "resource://gre/modules/osfile.jsm", true);
+
+function getHeapSnapshotFileTemplate() {
+  return OS.Path.join(OS.Constants.Path.tmpDir, `${Date.now()}.fxsnapshot`);
+}
+
+/**
+ * Get a unique temp file path for a new heap snapshot. The file is guaranteed
+ * not to exist before this call.
+ *
+ * @returns String
+ */
+exports.getNewUniqueHeapSnapshotTempFilePath = function () {
+  let file = new FileUtils.File(getHeapSnapshotFileTemplate());
+  // The call to createUnique will append "-N" after the leaf name (but before
+  // the extension) until a new file is found and create it. This guarantees we
+  // won't accidentally choose the same file twice.
+  file.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0o666);
+  return file.path;
+};
+
+function isValidSnapshotFileId(snapshotId) {
+  return /^\d+(\-\d+)?$/.test(snapshotId);
+}
+
+/**
+ * Get the file path for the given snapshot id.
+ *
+ * @param {String} snapshotId
+ *
+ * @returns String | null
+ */
+exports.getHeapSnapshotTempFilePath = function (snapshotId) {
+  // Don't want anyone sneaking "../../../.." strings into the snapshot id and
+  // trying to make us open arbitrary files.
+  if (!isValidSnapshotFileId(snapshotId)) {
+    return null;
+  }
+  return OS.Path.join(OS.Constants.Path.tmpDir, snapshotId + ".fxsnapshot");
+};
+
+/**
+ * Return true if we have the heap snapshot file for the given snapshot id on
+ * the local file system. False is returned otherwise.
+ *
+ * @returns Promise<Boolean>
+ */
+exports.haveHeapSnapshotTempFile = function (snapshotId) {
+  const path = exports.getHeapSnapshotTempFilePath(snapshotId);
+  if (!path) {
+    return Promise.resolve(false);
+  }
+
+  return OS.File.stat(path).then(() => true,
+                                 () => false);
+};
+
+/**
+ * Given a heap snapshot's file path, extricate the snapshot id.
+ *
+ * @param {String} path
+ *
+ * @returns String
+ */
+exports.getSnapshotIdFromPath = function (path) {
+  return path.slice(OS.Constants.Path.tmpDir.length + 1,
+                    path.length - ".fxsnapshot".length);
+};

toolkit/devtools/heapsnapshot/moz.build

@@ -35,4 +35,5 @@ EXTRA_JS_MODULES.devtools.heapsnapshot += [
     'census-tree-node.js',
     'HeapAnalysesClient.js',
     'HeapAnalysesWorker.js',
+    'HeapSnapshotFileUtils.js',
 ]

toolkit/devtools/server/actors/memory.js

@@ -4,6 +4,8 @@
 
 "use strict";
 
+const { Cc, Ci, Cu, components } = require("chrome");
+const { openFileStream } = require("devtools/toolkit/DevToolsUtils");
 const protocol = require("devtools/server/protocol");
 const { method, RetVal, Arg, types } = protocol;
 const { Memory } = require("devtools/toolkit/shared/memory");
@@ -11,6 +13,14 @@ const { actorBridge } = require("devtools/server/actors/common");
 loader.lazyRequireGetter(this, "events", "sdk/event/core");
 loader.lazyRequireGetter(this, "StackFrameCache",
                          "devtools/server/actors/utils/stack", true);
+loader.lazyRequireGetter(this, "FileUtils",
+                         "resource://gre/modules/FileUtils.jsm", true);
+loader.lazyRequireGetter(this, "NetUtil", "resource://gre/modules/NetUtil.jsm", true);
+loader.lazyRequireGetter(this, "Task", "resource://gre/modules/Task.jsm", true);
+loader.lazyRequireGetter(this, "OS", "resource://gre/modules/osfile.jsm", true);
+loader.lazyRequireGetter(this, "HeapSnapshotFileUtils",
+                         "devtools/toolkit/heapsnapshot/HeapSnapshotFileUtils");
+loader.lazyRequireGetter(this, "ThreadSafeChromeUtils");
 
 types.addDictType("AllocationsRecordingOptions", {
   // The probability we sample any given allocation when recording
@@ -97,6 +107,43 @@ let MemoryActor = exports.MemoryActor = protocol.ActorClass({
     }
   }),
 
+  saveHeapSnapshot: method(function () {
+    return this.bridge.saveHeapSnapshot();
+  }, {
+    response: {
+      snapshotId: RetVal("string")
+    }
+  }),
+
+  transferHeapSnapshot: method(Task.async(function* (snapshotId) {
+    const snapshotFilePath =
+      HeapSnapshotFileUtils.getHeapSnapshotTempFilePath(snapshotId);
+    if (!snapshotFilePath) {
+      throw new Error(`No heap snapshot with id: ${snapshotId}`);
+    }
+
+    const streamPromise = openFileStream(snapshotFilePath);
+
+    const { size } = yield OS.File.stat(snapshotFilePath);
+    const bulkPromise = this.conn.startBulkSend({
+      actor: this.actorID,
+      type: "heap-snapshot",
+      length: size
+    });
+
+    const [bulk, stream] = yield Promise.all([bulkPromise, streamPromise]);
+
+    try {
+      yield bulk.copyFrom(stream);
+    } finally {
+      stream.close();
+    }
+  }), {
+    request: {
+      snapshotId: Arg(0, "string")
+    }
+  }),
+
   takeCensus: actorBridge("takeCensus", {
     request: {},
     response: RetVal("json")
@@ -153,18 +200,81 @@ let MemoryActor = exports.MemoryActor = protocol.ActorClass({
   }),
 
   _onGarbageCollection: function (data) {
-    events.emit(this, "garbage-collection", data);
+    if (this.conn.transport) {
+      events.emit(this, "garbage-collection", data);
+    }
   },
 
   _onAllocations: function (data) {
-    events.emit(this, "allocations", data);
+    if (this.conn.transport) {
+      events.emit(this, "allocations", data);
+    }
   },
 });
 
 exports.MemoryFront = protocol.FrontClass(MemoryActor, {
   initialize: function(client, form) {
     protocol.Front.prototype.initialize.call(this, client, form);
+    this._client = client;
     this.actorID = form.memoryActor;
     this.manage(this);
-  }
+  },
+
+  /**
+   * Save a heap snapshot, transfer it from the server to the client if the
+   * server and client do not share a file system, and return the local file
+   * path to the heap snapshot.
+   *
+   * NB: This will not work with sandboxed child processes, as they do not have
+   * access to the filesystem and the hep snapshot APIs do not support that use
+   * case yet.
+   *
+   * @params Boolean options.forceCopy
+   *         Always force a bulk data copy of the saved heap snapshot, even when
+   *         the server and client share a file system.
+   *
+   * @returns Promise<String>
+   */
+  saveHeapSnapshot: protocol.custom(Task.async(function* (options = {}) {
+    const snapshotId = yield this._saveHeapSnapshotImpl();
+
+    if (!options.forceCopy &&
+        (yield HeapSnapshotFileUtils.haveHeapSnapshotTempFile(snapshotId))) {
+      return HeapSnapshotFileUtils.getHeapSnapshotTempFilePath(snapshotId);
+    }
+
+    return yield this.transferHeapSnapshot(snapshotId);
+  }), {
+    impl: "_saveHeapSnapshotImpl"
+  }),
+
+  /**
+   * Given that we have taken a heap snapshot with the given id, transfer the
+   * heap snapshot file to the client. The path to the client's local file is
+   * returned.
+   *
+   * @param {String} snapshotId
+   *
+   * @returns Promise<String>
+   */
+  transferHeapSnapshot: protocol.custom(function (snapshotId) {
+    const request = this._client.request({
+      to: this.actorID,
+      type: "transferHeapSnapshot",
+      snapshotId
+    });
+
+    return new Promise((resolve, reject) => {
+      const outFilePath =
+        HeapSnapshotFileUtils.getNewUniqueHeapSnapshotTempFilePath();
+      const outFile = new FileUtils.File(outFilePath);
+
+      const outFileStream = FileUtils.openSafeFileOutputStream(outFile);
+      request.on("bulk-reply", Task.async(function* ({ copyTo }) {
+        yield copyTo(outFileStream);
+        FileUtils.closeSafeFileOutputStream(outFileStream);
+        resolve(outFilePath);
+      }));
+    });
+  })
 });

toolkit/devtools/server/tests/unit/head_dbg.js

@@ -8,7 +8,7 @@ const Cu = Components.utils;
 const Cr = Components.results;
 const CC = Components.Constructor;
 
-const { require } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
+const { require, loader } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {});
 const { worker } = Cu.import("resource://gre/modules/devtools/worker-loader.js", {})
 const promise = require("promise");
 const { Task } = Cu.import("resource://gre/modules/Task.jsm", {});
@@ -25,6 +25,7 @@ const DevToolsUtils = require("devtools/toolkit/DevToolsUtils.js");
 const { DebuggerServer } = require("devtools/server/main");
 const { DebuggerServer: WorkerDebuggerServer } = worker.require("devtools/server/main");
 const { DebuggerClient, ObjectClient } = require("devtools/toolkit/client/main");
+const { MemoryFront } = require("devtools/server/actors/memory");
 
 const { addDebuggerToGlobal } = Cu.import("resource://gre/modules/jsdebugger.jsm", {});
 
@@ -34,6 +35,42 @@ let loadSubScript = Cc[
   '@mozilla.org/moz/jssubscript-loader;1'
 ].getService(Ci.mozIJSSubScriptLoader).loadSubScript;
 
+/**
+ * Create a `run_test` function that runs the given generator in a task after
+ * having attached to a memory actor. When done, the memory actor is detached
+ * from, the client is finished, and the test is finished.
+ *
+ * @param {GeneratorFunction} testGeneratorFunction
+ *        The generator function is passed (DebuggerClient, MemoryFront)
+ *        arguments.
+ *
+ * @returns `run_test` function
+ */
+function makeMemoryActorTest(testGeneratorFunction) {
+  const TEST_GLOBAL_NAME = "test_MemoryActor";
+
+  return function run_test() {
+    do_test_pending();
+    startTestDebuggerServer(TEST_GLOBAL_NAME).then(client => {
+      getTestTab(client, TEST_GLOBAL_NAME, function (tabForm) {
+        Task.spawn(function* () {
+          try {
+            const memoryFront = new MemoryFront(client, tabForm);
+            yield memoryFront.attach();
+            yield* testGeneratorFunction(client, memoryFront);
+            yield memoryFront.detach();
+          } catch(err) {
+            DevToolsUtils.reportException("makeMemoryActorTest", err);
+            ok(false, "Got an error: " + err);
+          }
+
+          finishClient(client);
+        });
+      });
+    });
+  };
+}
+
 function createTestGlobal(name) {
   let sandbox = Cu.Sandbox(
     Cc["@mozilla.org/systemprincipal;1"].createInstance(Ci.nsIPrincipal)
@@ -176,41 +213,47 @@ function dbg_assert(cond, e) {
 let errorCount = 0;
 let listener = {
   observe: function (aMessage) {
-    errorCount++;
     try {
-      // If we've been given an nsIScriptError, then we can print out
-      // something nicely formatted, for tools like Emacs to pick up.
-      var scriptError = aMessage.QueryInterface(Ci.nsIScriptError);
-      dumpn(aMessage.sourceName + ":" + aMessage.lineNumber + ": " +
-            scriptErrorFlagsToKind(aMessage.flags) + ": " +
-            aMessage.errorMessage);
-      var string = aMessage.errorMessage;
-    } catch (x) {
-      // Be a little paranoid with message, as the whole goal here is to lose
-      // no information.
+      errorCount++;
       try {
-        var string = "" + aMessage.message;
+        // If we've been given an nsIScriptError, then we can print out
+        // something nicely formatted, for tools like Emacs to pick up.
+        var scriptError = aMessage.QueryInterface(Ci.nsIScriptError);
+        dumpn(aMessage.sourceName + ":" + aMessage.lineNumber + ": " +
+              scriptErrorFlagsToKind(aMessage.flags) + ": " +
+              aMessage.errorMessage);
+        var string = aMessage.errorMessage;
       } catch (x) {
-        var string = "<error converting error message to string>";
+        // Be a little paranoid with message, as the whole goal here is to lose
+        // no information.
+        try {
+          var string = "" + aMessage.message;
+        } catch (x) {
+          var string = "<error converting error message to string>";
+        }
       }
-    }
 
-    // Make sure we exit all nested event loops so that the test can finish.
-    while (DebuggerServer.xpcInspector
-           && DebuggerServer.xpcInspector.eventLoopNestLevel > 0) {
-      DebuggerServer.xpcInspector.exitNestedEventLoop();
-    }
+      // Make sure we exit all nested event loops so that the test can finish.
+      while (DebuggerServer
+             && DebuggerServer.xpcInspector
+             && DebuggerServer.xpcInspector.eventLoopNestLevel > 0) {
+        DebuggerServer.xpcInspector.exitNestedEventLoop();
+      }
 
-    // In the world before bug 997440, exceptions were getting lost because of
-    // the arbitrary JSContext being used in nsXPCWrappedJSClass::CallMethod.
-    // In the new world, the wanderers have returned. However, because of the,
-    // currently very-broken, exception reporting machinery in XPCWrappedJSClass
-    // these get reported as errors to the console, even if there's actually JS
-    // on the stack above that will catch them.
-    // If we throw an error here because of them our tests start failing.
-    // So, we'll just dump the message to the logs instead, to make sure the
-    // information isn't lost.
-    dumpn("head_dbg.js observed a console message: " + string);
+      // In the world before bug 997440, exceptions were getting lost because of
+      // the arbitrary JSContext being used in nsXPCWrappedJSClass::CallMethod.
+      // In the new world, the wanderers have returned. However, because of the,
+      // currently very-broken, exception reporting machinery in
+      // XPCWrappedJSClass these get reported as errors to the console, even if
+      // there's actually JS on the stack above that will catch them.  If we
+      // throw an error here because of them our tests start failing.  So, we'll
+      // just dump the message to the logs instead, to make sure the information
+      // isn't lost.
+      dumpn("head_dbg.js observed a console message: " + string);
+    } catch (_) {
+      // Swallow everything to avoid console reentrancy errors. We did our best
+      // to log above, but apparently that didn't cut it.
+    }
   }
 };
 

toolkit/devtools/server/tests/unit/test_MemoryActor_saveHeapSnapshot_01.js

@@ -0,0 +1,16 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that we can tell the memory actor to take a heap snapshot over the RDP
+// and then create a HeapSnapshot instance from the resulting file.
+
+Cu.import("resource://gre/modules/osfile.jsm");
+
+const run_test = makeMemoryActorTest(function* (client, memoryFront) {
+  const snapshotFilePath = yield memoryFront.saveHeapSnapshot();
+  ok(!!(yield OS.File.stat(snapshotFilePath)),
+     "Should have the heap snapshot file");
+  const snapshot = ThreadSafeChromeUtils.readHeapSnapshot(snapshotFilePath);
+  ok(snapshot instanceof HeapSnapshot,
+     "And we should be able to read a HeapSnapshot instance from the file");
+});

toolkit/devtools/server/tests/unit/test_MemoryActor_saveHeapSnapshot_02.js

@@ -0,0 +1,18 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Test that we can properly stream heap snapshot files over the RDP as bulk
+// data.
+
+Cu.import("resource://gre/modules/osfile.jsm");
+
+const run_test = makeMemoryActorTest(function* (client, memoryFront) {
+  const snapshotFilePath = yield memoryFront.saveHeapSnapshot({
+    forceCopy: true
+  });
+  ok(!!(yield OS.File.stat(snapshotFilePath)),
+     "Should have the heap snapshot file");
+  const snapshot = ThreadSafeChromeUtils.readHeapSnapshot(snapshotFilePath);
+  ok(snapshot instanceof HeapSnapshot,
+     "And we should be able to read a HeapSnapshot instance from the file");
+});

toolkit/devtools/server/tests/unit/xpcshell.ini

@@ -44,6 +44,8 @@ support-files =
 [test_dbgglobal.js]
 [test_dbgclient_debuggerstatement.js]
 [test_attach.js]
+[test_MemoryActor_saveHeapSnapshot_01.js]
+[test_MemoryActor_saveHeapSnapshot_02.js]
 [test_reattach-thread.js]
 [test_blackboxing-01.js]
 [test_blackboxing-02.js]

toolkit/devtools/shared/memory.js

@@ -14,6 +14,9 @@ loader.lazyRequireGetter(this, "DeferredTask",
   "resource://gre/modules/DeferredTask.jsm", true);
 loader.lazyRequireGetter(this, "StackFrameCache",
   "devtools/server/actors/utils/stack", true);
+loader.lazyRequireGetter(this, "ThreadSafeChromeUtils");
+loader.lazyRequireGetter(this, "HeapSnapshotFileUtils",
+  "devtools/toolkit/heapsnapshot/HeapSnapshotFileUtils");
 
 /**
  * A class that returns memory data for a parent actor's window.
@@ -62,7 +65,6 @@ let Memory = exports.Memory = Class({
     return this._dbg;
   },
 
-
   /**
    * Attach to this MemoryBridge.
    *
@@ -130,6 +132,18 @@ let Memory = exports.Memory = Class({
     return this.dbg.memory.trackingAllocationSites;
   },
 
+  /**
+   * Save a heap snapshot scoped to the current debuggees' portion of the heap
+   * graph.
+   *
+   * @returns {String} The snapshot id.
+   */
+  saveHeapSnapshot: expectState("attached", function () {
+    const path = HeapSnapshotFileUtils.getNewUniqueHeapSnapshotTempFilePath();
+    ThreadSafeChromeUtils.saveHeapSnapshot(path, { debugger: this.dbg });
+    return HeapSnapshotFileUtils.getSnapshotIdFromPath(path);
+  }, "saveHeapSnapshot"),
+
   /**
    * Take a census of the heap. See js/src/doc/Debugger/Debugger.Memory.md for
    * more information.
@@ -146,8 +160,8 @@ let Memory = exports.Memory = Class({
    *                 Must be between 0 and 1 -- defaults to 1.
    * @param {number} options.maxLogLength
    *                 The maximum number of allocation events to keep in the
-   *                 log. If new allocs occur while at capacity, oldest allocs are lost.
-   *                 Must fit in a 32 bit signed integer.
+   *                 log. If new allocs occur while at capacity, oldest
+   *                 allocations are lost. Must fit in a 32 bit signed integer.
    * @param {number} options.drainAllocationsTimeout
    *                 A number in milliseconds of how often, at least, an `allocation` event
    *                 gets emitted (and drained), and also emits and drains on every GC event,