From d4cdedeed70c0fee4f74d680102a4c5ef147e118 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 19 May 2022 17:41:20 +0100 Subject: [PATCH 1/3] Set status to ENABLING when not connected to any guards/bridges. --- .../org/briarproject/bramble/plugin/tor/TorPlugin.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 aa0fff972..5f7ca3293 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 @@ -1071,7 +1071,8 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { if (reasonsDisabled != 0) return DISABLED; if (!networkInitialised) return ENABLING; if (!networkEnabled) return INACTIVE; - return bootstrapped && circuitBuilt ? ACTIVE : ENABLING; + return bootstrapped && circuitBuilt && orConnectionsConnected > 0 + ? ACTIVE : ENABLING; } private synchronized int getReasonsDisabled() { @@ -1098,17 +1099,23 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { LOG.warning("Count was zero before connection connected"); orConnectionsPending = 0; } + int oldConnected = orConnectionsConnected; orConnectionsConnected++; logOrConnections(); + if (oldConnected == 0) callback.pluginStateChanged(getState()); } private synchronized void onOrConnectionClosed() { + int oldConnected = orConnectionsConnected; orConnectionsConnected--; if (orConnectionsConnected < 0) { LOG.warning("Count was zero before connection closed"); orConnectionsConnected = 0; } logOrConnections(); + if (orConnectionsConnected == 0 && oldConnected != 0) { + callback.pluginStateChanged(getState()); + } } private synchronized void onSwitchingGuardContext() { From 0b85aca932fb7d606061c700ff60d600bb8f035b Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 19 May 2022 17:45:10 +0100 Subject: [PATCH 2/3] Remove connectivity workaround that should no longer be needed. --- .../bramble/plugin/tor/TorPlugin.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 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 5f7ca3293..8914979d3 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 @@ -751,13 +751,6 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { else if (status.equals("FAILED")) state.onOrConnectionFailed(); else if (status.equals("CONNECTED")) state.onOrConnectionConnected(); else if (status.equals("CLOSED")) state.onOrConnectionClosed(); - - if ((status.equals("FAILED") || status.equals("CLOSED")) && - state.getNumOrConnections() == 0) { - // Check whether we've lost connectivity - updateConnectionStatus(networkManager.getNetworkStatus(), - batteryManager.isCharging()); - } } @Override @@ -857,8 +850,8 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { settings = s.getSettings(); // Works around a bug introduced in Tor 0.3.4.8. // https://trac.torproject.org/projects/tor/ticket/28027 - // Could be replaced with callback.transportDisabled() - // when fixed. + // 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()); @@ -1014,6 +1007,7 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { callback.pluginStateChanged(getState()); } + @SuppressWarnings("BooleanMethodIsAlwaysInverted") private synchronized boolean isTorRunning() { return started && !stopped; } @@ -1126,10 +1120,6 @@ abstract class TorPlugin implements DuplexPlugin, EventHandler, EventListener { logOrConnections(); } - private synchronized int getNumOrConnections() { - return orConnectionsPending + orConnectionsConnected; - } - @GuardedBy("this") private void logOrConnections() { if (LOG.isLoggable(INFO)) { From f387c3801b590fe763607020f07f1e11b19fa8cc Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 20 May 2022 15:48:25 +0100 Subject: [PATCH 3/3] 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"); } } }