[tbb-commits] [Git][tpo/applications/tor-browser][tor-browser-102.9.0esr-12.5-1] fixup! Bug 40933: Add tor-launcher functionality

[Pier Angelo Vendrame (@pierov)]

Pier Angelo Vendrame pushed to branch tor-browser-102.9.0esr-12.5-1 at The Tor Project / Applications / Tor Browser


Commits:
6dfea410 by Pier Angelo Vendrame at 2023-04-04T18:32:53+02:00
fixup! Bug 40933: Add tor-launcher functionality

Bug 41709: Reimplement the failure logic for
TorProtocolService.sendCommand

sendCommand tries to send a command forever every 250ms in case of
failure. That is bad because some async code branch might be stuck in it
forever.

We should have a number of maximum attempts and increase the delay after
which we try again.

- - - - -


2 changed files:

- toolkit/components/tor-launcher/TorMonitorService.jsm
- toolkit/components/tor-launcher/TorProtocolService.jsm


Changes:

=====================================
toolkit/components/tor-launcher/TorMonitorService.jsm
=====================================
@@ -233,8 +233,8 @@ const TorMonitorService = {
 
         // FIXME: TorProcess is misleading here. We should use a topic related
         // to having a control port connection, instead.
+        logger.info(`Notifying ${TorTopics.ProcessIsReady}`);
         Services.obs.notifyObservers(null, TorTopics.ProcessIsReady);
-        logger.info(`Notified ${TorTopics.ProcessIsReady}`);
 
         // We reset this here hoping that _shutDownEventMonitor can interrupt
         // the current monitor, either by calling clearTimeout and preventing it


=====================================
toolkit/components/tor-launcher/TorProtocolService.jsm
=====================================
@@ -220,49 +220,29 @@ const TorProtocolService = {
   // Executes a command on the control port.
   // Return a reply object or null if a fatal error occurs.
   async sendCommand(cmd, args) {
-    let conn, reply;
-    const maxAttempts = 2;
-    for (let attempt = 0; !reply && attempt < maxAttempts; attempt++) {
-      try {
-        conn = await this._getConnection();
-        try {
-          if (conn) {
-            reply = await conn.sendCommand(cmd + (args ? " " + args : ""));
-            if (reply) {
-              // Return for reuse.
-              this._returnConnection();
-            } else {
-              // Connection is bad.
-              logger.warn(
-                "sendCommand returned an empty response, taking the connection as broken and closing it."
-              );
-              this._closeConnection();
-            }
-          }
-        } catch (e) {
-          logger.error(`Cannot send the command ${cmd}`, e);
-          this._closeConnection();
-        }
-      } catch (e) {
-        logger.error("Cannot get a connection to the control port", e);
+    const maxTimeout = 1000;
+    let leftConnAttempts = 5;
+    let timeout = 250;
+    let reply;
+    while (leftConnAttempts-- > 0) {
+      const response = await this._trySend(cmd, args, leftConnAttempts == 0);
+      if (response.connected) {
+        reply = response.reply;
+        break;
       }
-    }
-
-    // We failed to acquire the controller after multiple attempts.
-    // Try again after some time.
-    if (!conn) {
-      logger.info(
-        "sendCommand: Acquiring control connection failed",
+      // We failed to acquire the controller after multiple attempts.
+      // Try again after some time.
+      logger.warn(
+        "sendCommand: Acquiring control connection failed, trying again later.",
         cmd,
         args
       );
-      return new Promise(resolve =>
-        setTimeout(() => {
-          resolve(this.sendCommand(cmd, args));
-        }, 250)
-      );
+      await new Promise(resolve => setTimeout(() => resolve(), timeout));
+      timeout = Math.min(2 * timeout, maxTimeout);
     }
 
+    // We sent the command, but we still got an empty response.
+    // Something must be busted elsewhere.
     if (!reply) {
       throw new Error(`${cmd} sent an empty response`);
     }
@@ -590,6 +570,48 @@ const TorProtocolService = {
     }
   },
 
+  async _trySend(cmd, args, rethrow) {
+    let connected = false;
+    let reply;
+    let leftAttempts = 2;
+    while (leftAttempts-- > 0) {
+      let conn;
+      try {
+        conn = await this._getConnection();
+      } catch (e) {
+        logger.error("Cannot get a connection to the control port", e);
+        if (leftAttempts == 0 && rethrow) {
+          throw e;
+        }
+      }
+      if (!conn) {
+        continue;
+      }
+      // If we _ever_ got a connection, the caller should not try again
+      connected = true;
+      try {
+        reply = await conn.sendCommand(cmd + (args ? " " + args : ""));
+        if (reply) {
+          // Return for reuse.
+          this._returnConnection();
+        } else {
+          // Connection is bad.
+          logger.warn(
+            "sendCommand returned an empty response, taking the connection as broken and closing it."
+          );
+          this._closeConnection();
+        }
+      } catch (e) {
+        logger.error(`Cannot send the command ${cmd}`, e);
+        this._closeConnection();
+        if (leftAttempts == 0 && rethrow) {
+          throw e;
+        }
+      }
+    }
+    return { connected, reply };
+  },
+
   // Opens an authenticated connection, sets it to this._controlConnection, and
   // return it.
   async _getConnection() {



View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/6dfea410c55818f7a62395ec34f799c7a85ad5b9

-- 
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/commit/6dfea410c55818f7a62395ec34f799c7a85ad5b9
You're receiving this email because of your account on gitlab.torproject.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tbb-commits/attachments/20230404/f6ad95f7/attachment-0001.htm>