Bug 1128027 - Clean up protocol.js pools after connection close. r=bgrins a=lsblakk
💩💩 backed out by e49cd895e078 💩 💩
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 27 Feb 2015 17:51:56 -0600
changeset 250126 021aac3d7804
parent 250125 20ea789e69df
child 250127 4c02cca13dbe
push id4508
push userjryans@gmail.com
push date2015-02-27 23:52 +0000
treeherdermozilla-beta@012e92feffe6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbgrins, lsblakk
bugs1128027
milestone37.0
Bug 1128027 - Clean up protocol.js pools after connection close. r=bgrins a=lsblakk
toolkit/devtools/client/dbg-client.jsm
toolkit/devtools/server/protocol.js
toolkit/devtools/server/tests/unit/test_protocol_abort.js
toolkit/devtools/server/tests/unit/xpcshell.ini
--- a/toolkit/devtools/client/dbg-client.jsm
+++ b/toolkit/devtools/client/dbg-client.jsm
@@ -1007,16 +1007,37 @@ DebuggerClient.prototype = {
    * Called by DebuggerTransport when the underlying stream is closed.
    *
    * @param aStatus nsresult
    *        The status code that corresponds to the reason for closing
    *        the stream.
    */
   onClosed: function (aStatus) {
     this.emit("closed");
+
+    // The |_pools| array on the client-side currently is used only by
+    // protocol.js to store active fronts, mirroring the actor pools found in
+    // the server.  So, read all usages of "pool" as "protocol.js front".
+    //
+    // In the normal case where we shutdown cleanly, the toolbox tells each tool
+    // to close, and they each call |destroy| on any fronts they were using.
+    // When |destroy| or |cleanup| is called on a protocol.js front, it also
+    // removes itself from the |_pools| array.  Once the toolbox has shutdown,
+    // the connection is closed, and we reach here.  All fronts (should have
+    // been) |destroy|ed, so |_pools| should empty.
+    //
+    // If the connection instead aborts unexpectedly, we may end up here with
+    // all fronts used during the life of the connection.  So, we call |cleanup|
+    // on them clear their state, reject pending requests, and remove themselves
+    // from |_pools|.  This saves the toolbox from hanging indefinitely, in case
+    // it waits for some server response before shutdown that will now never
+    // arrive.
+    for (let pool of this._pools) {
+      pool.cleanup();
+    }
   },
 
   registerClient: function (client) {
     let actorID = client.actor;
     if (!actorID) {
       throw new Error("DebuggerServer.registerClient expects " +
                       "a client instance with an `actor` attribute.");
     }
--- a/toolkit/devtools/server/protocol.js
+++ b/toolkit/devtools/server/protocol.js
@@ -1105,17 +1105,17 @@ let Front = Class({
     }
   },
 
   destroy: function() {
     // Reject all outstanding requests, they won't make sense after
     // the front is destroyed.
     while (this._requests && this._requests.length > 0) {
       let deferred = this._requests.shift();
-      deferred.reject(new Error("Connection closed"));
+      deferred.reject(new Error("Connection closed, pending request not sent"));
     }
     Pool.prototype.destroy.call(this);
     this.actorID = null;
   },
 
   manage: function(front) {
     if (!front.actorID) {
       throw new Error("Can't manage front without an actor ID.\n" +
new file mode 100644
--- /dev/null
+++ b/toolkit/devtools/server/tests/unit/test_protocol_abort.js
@@ -0,0 +1,72 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Outstanding requests should be rejected when the connection aborts
+ * unexpectedly.
+ */
+
+let protocol = devtools.require("devtools/server/protocol");
+let {method, Arg, Option, RetVal} = protocol;
+let events = devtools.require("sdk/event/core");
+
+function simpleHello() {
+  return {
+    from: "root",
+    applicationType: "xpcshell-tests",
+    traits: [],
+  }
+}
+
+let RootActor = protocol.ActorClass({
+  typeName: "root",
+  initialize: function(conn) {
+    protocol.Actor.prototype.initialize.call(this, conn);
+    // Root actor owns itself.
+    this.manage(this);
+    this.actorID = "root";
+    this.sequence = 0;
+  },
+
+  sayHello: simpleHello,
+
+  simpleReturn: method(function() {
+    return this.sequence++;
+  }, {
+    response: { value: RetVal() },
+  })
+});
+
+let RootFront = protocol.FrontClass(RootActor, {
+  initialize: function(client) {
+    this.actorID = "root";
+    protocol.Front.prototype.initialize.call(this, client);
+    // Root owns itself.
+    this.manage(this);
+  }
+});
+
+function run_test() {
+  DebuggerServer.createRootActor = RootActor;
+  DebuggerServer.init();
+
+  let trace = connectPipeTracing();
+  let client = new DebuggerClient(trace);
+  let rootClient;
+
+  client.connect((applicationType, traits) => {
+    rootClient = RootFront(client);
+
+    rootClient.simpleReturn().then(() => {
+      ok(false, "Connection was aborted, request shouldn't resolve");
+      do_test_finished();
+    }, () => {
+      ok(true, "Connection was aborted, request rejected correctly");
+      do_test_finished();
+    });
+
+    trace.close();
+  });
+
+  do_test_pending();
+}
--- a/toolkit/devtools/server/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/server/tests/unit/xpcshell.ini
@@ -59,16 +59,17 @@ support-files =
 [test_frameclient-02.js]
 [test_nativewrappers.js]
 [test_nodelistactor.js]
 [test_eval-01.js]
 [test_eval-02.js]
 [test_eval-03.js]
 [test_eval-04.js]
 [test_eval-05.js]
+[test_protocol_abort.js]
 [test_protocol_async.js]
 [test_protocol_children.js]
 [test_protocol_formtype.js]
 [test_protocol_longstring.js]
 [test_protocol_simple.js]
 [test_protocol_unregister.js]
 [test_breakpoint-01.js]
 [test_register_actor.js]