From a30223fe5ed34c3a3d469206c117235a313e2753 Mon Sep 17 00:00:00 2001
From: Andreas Tolfsen <ato@sny.no>
Date: Tue, 19 Nov 2019 12:32:39 +0000
Subject: [PATCH] bug 1543115: remote: make RemoteAgent.close() safer;
 r=remote-protocol-reviewers,maja_zf

close() is meant to be failsafe in the sense that it should be
possible to call without side-effects.

We are currently setting up a lot of state in listen() that is not
cleaned up if the server eventually fails to start.  Calling close()
when this happens will ensure any state listen() has accrued is reset.

Differential Revision: https://phabricator.services.mozilla.com/D50282

--HG--
extra : moz-landing-system : lando
---
 remote/RemoteAgent.jsm | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/remote/RemoteAgent.jsm b/remote/RemoteAgent.jsm
index e186e19e7f54..3817bcc34b48 100644
--- a/remote/RemoteAgent.jsm
+++ b/remote/RemoteAgent.jsm
@@ -90,6 +90,7 @@ class RemoteAgentClass {
       this.server._start(port, host);
       dump(`DevTools listening on ${mainTarget.wsDebuggerURL}\n`);
     } catch (e) {
+      await this.close();
       throw new Error(`Unable to start remote agent: ${e.message}`, e);
     }
 
@@ -97,20 +98,24 @@ class RemoteAgentClass {
   }
 
   async close() {
-    if (this.listening) {
-      try {
-        // Destroy all the targets first in order to ensure closing all pending
-        // connections first. Otherwise Httpd's stop is not going to resolve.
+    try {
+      Preferences.reset(Object.keys(RecommendedPreferences));
+
+      // destroy targets before stopping server,
+      // otherwise the HTTP will fail to stop
+      if (this.targets) {
         this.targets.destructor();
-
-        await this.server.stop();
-
-        Preferences.reset(Object.keys(RecommendedPreferences));
-      } catch (e) {
-        throw new Error(`Unable to stop agent: ${e.message}`, e);
       }
 
-      log.info("Stopped listening");
+      if (this.listening) {
+        await this.server.stop();
+      }
+    } catch (e) {
+      // this function must never fail
+      log.error("unable to stop listener", e);
+    } finally {
+      this.server = null;
+      this.targets = null;
     }
   }