Bug 1646456 - Only call destroy on top level pools from connection onClosed r=ochameau,nchevobbe
authorJulian Descottes <jdescottes@mozilla.com>
Tue, 23 Jun 2020 11:08:30 +0000
changeset 600949 38c2fa230269c6e1d0f6022cfb578b790cf2e6f5
parent 600948 fdabfde164e98448cd76dc229e204b1749f0a00c
child 600950 6fc6ec584f86d227413e2012f2cc574564f2e49d
push id13310
push userffxbld-merge
push dateMon, 29 Jun 2020 14:50:06 +0000
treeherdermozilla-beta@15a59a0afa5c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersochameau, nchevobbe
bugs1646456
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1646456 - Only call destroy on top level pools from connection onClosed r=ochameau,nchevobbe Depends on D80060 Differential Revision: https://phabricator.services.mozilla.com/D80064
devtools/server/actors/common.js
devtools/server/devtools-server-connection.js
devtools/shared/protocol/Pool.js
--- a/devtools/server/actors/common.js
+++ b/devtools/server/actors/common.js
@@ -111,16 +111,22 @@ ActorPool.prototype = {
     }
   },
 
   dumpPool() {
     for (const actor in this._actors) {
       console.log(`>> ${actor}`);
     }
   },
+
+  // For consistency with Pool.js
+  // Will be checked by devtools-server-connection.
+  isTopPool() {
+    return true;
+  },
 };
 
 exports.ActorPool = ActorPool;
 
 /**
  * A SourceLocation represents a location in a source.
  *
  * @param SourceActor actor
--- a/devtools/server/devtools-server-connection.js
+++ b/devtools/server/devtools-server-connection.js
@@ -471,17 +471,26 @@ DevToolsServerConnection.prototype = {
     if (!this._actorPool) {
       // Ignore this call if the connection is already closed.
       return;
     }
     this._actorPool = null;
 
     this.emit("closed", status, this.prefix);
 
-    this._extraPools.forEach(p => p.destroy());
+    // Use filter in order to create a copy of the extraPools array,
+    // which might be modified by removeActorPool calls.
+    // The isTopLevel check ensures that the pools retrieved here will not be
+    // destroyed by another Pool::destroy. Non top-level pools will be destroyed
+    // by the recursive Pool::destroy mechanism.
+    // See test_connection_closes_all_pools.js for practical examples of Pool
+    // hierarchies.
+    const topLevelPools = this._extraPools.filter(p => p.isTopPool());
+    topLevelPools.forEach(p => p.destroy());
+
     this._extraPools = null;
 
     this.rootActor = null;
     this._transport = null;
     DevToolsServer._connectionClosed(this);
   },
 
   dumpPool(pool, output = [], dumpedPools) {
--- a/devtools/shared/protocol/Pool.js
+++ b/devtools/shared/protocol/Pool.js
@@ -35,16 +35,26 @@ class Pool extends EventEmitter {
 
   /**
    * Return the parent pool for this client.
    */
   getParent() {
     return this.conn.poolFor(this.actorID);
   }
 
+  /**
+   * A pool is at the top of its pool hierarchy if it has:
+   * - no parent
+   * - or it is its own parent
+   */
+  isTopPool() {
+    const parent = this.getParent();
+    return !parent || parent === this;
+  }
+
   poolFor(actorID) {
     return this.conn.poolFor(actorID);
   }
 
   /**
    * Override this if you want actors returned by this actor
    * to belong to a different actor by default.
    */