From f387c3801b590fe763607020f07f1e11b19fa8cc Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Fri, 20 May 2022 15:48:25 +0100
Subject: [PATCH] Don't count pending OR connections, don't reset connection
 count.

Tor doesn't report status changes for bridge connections that fail during handshaking, which causes the pending connection count to become inaccurate.

We were resetting the connection counts when switching guard context, but this was a mistake caused by the pending connection count being inaccurate. The counts should not be reset, as Tor continues to report status changes for connected connections belonging to the old context.

It's no longer necessary to disable and re-enable the network when the Tor settings are updated. This only appeared to be necessary because we were wrongly resetting the connection counts.
---
 .../bramble/plugin/tor/TorPlugin.java         | 55 +------------------
 1 file changed, 3 insertions(+), 52 deletions(-)

diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java
index 8914979d3..6ce335a77 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/tor/TorPlugin.java
@@ -746,10 +746,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	public void orConnStatus(String status, String orName) {
 		if (LOG.isLoggable(INFO)) LOG.info("OR connection " + status);
 
-		//noinspection IfCanBeSwitch
-		if (status.equals("LAUNCHED")) state.onOrConnectionLaunched();
-		else if (status.equals("FAILED")) state.onOrConnectionFailed();
-		else if (status.equals("CONNECTED")) state.onOrConnectionConnected();
+		if (status.equals("CONNECTED")) state.onOrConnectionConnected();
 		else if (status.equals("CLOSED")) state.onOrConnectionClosed();
 	}
 
@@ -764,9 +761,6 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 	@Override
 	public void message(String severity, String msg) {
 		if (LOG.isLoggable(INFO)) LOG.info(severity + " " + msg);
-		if (msg.startsWith("Switching to guard context")) {
-			state.onSwitchingGuardContext();
-		}
 	}
 
 	@Override
@@ -848,11 +842,6 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			if (s.getNamespace().equals(ID.getString())) {
 				LOG.info("Tor settings updated");
 				settings = s.getSettings();
-				// Works around a bug introduced in Tor 0.3.4.8.
-				// https://trac.torproject.org/projects/tor/ticket/28027
-				// TODO: The upstream ticket is marked as fixed and backported,
-				//  but this workaround is still needed when disabling bridges
-				disableNetwork();
 				updateConnectionStatus(networkManager.getNetworkStatus(),
 						batteryManager.isCharging());
 			}
@@ -865,16 +854,6 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		}
 	}
 
-	private void disableNetwork() {
-		connectionStatusExecutor.execute(() -> {
-			try {
-				if (state.isTorRunning()) enableNetwork(false);
-			} catch (IOException ex) {
-				logException(LOG, WARNING, ex);
-			}
-		});
-	}
-
 	private void updateConnectionStatus(NetworkStatus status,
 			boolean charging) {
 		connectionStatusExecutor.execute(() -> {
@@ -1000,7 +979,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 		private ServerSocket serverSocket = null;
 
 		@GuardedBy("this")
-		private int orConnectionsPending = 0, orConnectionsConnected = 0;
+		private int orConnectionsConnected = 0;
 
 		private synchronized void setStarted() {
 			started = true;
@@ -1073,26 +1052,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			return getState() == DISABLED ? reasonsDisabled : 0;
 		}
 
-		private synchronized void onOrConnectionLaunched() {
-			orConnectionsPending++;
-			logOrConnections();
-		}
-
-		private synchronized void onOrConnectionFailed() {
-			orConnectionsPending--;
-			if (orConnectionsPending < 0) {
-				LOG.warning("Count was zero before connection failed");
-				orConnectionsPending = 0;
-			}
-			logOrConnections();
-		}
-
 		private synchronized void onOrConnectionConnected() {
-			orConnectionsPending--;
-			if (orConnectionsPending < 0) {
-				LOG.warning("Count was zero before connection connected");
-				orConnectionsPending = 0;
-			}
 			int oldConnected = orConnectionsConnected;
 			orConnectionsConnected++;
 			logOrConnections();
@@ -1112,19 +1072,10 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener {
 			}
 		}
 
-		private synchronized void onSwitchingGuardContext() {
-			// Tor doesn't seem to report events for connections belonging to
-			// the old guard context, so we have to reset the counters
-			orConnectionsPending = 0;
-			orConnectionsConnected = 0;
-			logOrConnections();
-		}
-
 		@GuardedBy("this")
 		private void logOrConnections() {
 			if (LOG.isLoggable(INFO)) {
-				LOG.info("OR connections: " + orConnectionsPending
-						+ " pending, " + orConnectionsConnected + " connected");
+				LOG.info(orConnectionsConnected + "OR connections connected");
 			}
 		}
 	}