From bda3b2100a60396fc3822dc101090b59d8ef7e54 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 5 May 2020 17:52:42 +0100 Subject: [PATCH] Raise the connection limit if connections are stable. --- .../AndroidBluetoothPluginFactory.java | 2 +- .../bluetooth/BluetoothConnectionLimiter.java | 7 ++ .../BluetoothConnectionLimiterImpl.java | 73 +++++++++++++--- .../BluetoothConnectionLimiterImplTest.java | 84 ++++++++++++++++++- .../bramble/plugin/DesktopPluginModule.java | 6 +- .../bluetooth/JavaBluetoothPluginFactory.java | 7 +- 6 files changed, 160 insertions(+), 19 deletions(-) diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java index bbf2e2744..3e911e772 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothPluginFactory.java @@ -67,7 +67,7 @@ public class AndroidBluetoothPluginFactory implements DuplexPluginFactory { @Override public DuplexPlugin createPlugin(PluginCallback callback) { BluetoothConnectionLimiter connectionLimiter = - new BluetoothConnectionLimiterImpl(); + new BluetoothConnectionLimiterImpl(clock); Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL, MAX_POLLING_INTERVAL, BACKOFF_BASE); AndroidBluetoothPlugin plugin = new AndroidBluetoothPlugin( diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java index 5061aae8b..993176bb7 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java @@ -3,9 +3,16 @@ package org.briarproject.bramble.plugin.bluetooth; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import static java.util.concurrent.TimeUnit.SECONDS; + @NotNullByDefault interface BluetoothConnectionLimiter { + /** + * How long a connection must remain open before it's considered stable. + */ + long STABILITY_PERIOD_MS = SECONDS.toMillis(90); + /** * Informs the limiter that key agreement has started. */ diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java index dbc530105..d08227834 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java @@ -2,15 +2,18 @@ package org.briarproject.bramble.plugin.bluetooth; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.system.Clock; import java.io.IOException; import java.util.ArrayList; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.logging.Logger; import javax.annotation.concurrent.GuardedBy; import javax.annotation.concurrent.ThreadSafe; +import javax.inject.Inject; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; @@ -24,21 +27,28 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { private static final Logger LOG = getLogger(BluetoothConnectionLimiterImpl.class.getName()); + private final Clock clock; + private final Object lock = new Object(); @GuardedBy("lock") - private final LinkedList connections = - new LinkedList<>(); + private final List connections = new LinkedList<>(); @GuardedBy("lock") private boolean keyAgreementInProgress = false; @GuardedBy("lock") private int connectionLimit = 1; + @Inject + BluetoothConnectionLimiterImpl(Clock clock) { + this.clock = clock; + } + @Override public void keyAgreementStarted() { List close; synchronized (lock) { keyAgreementInProgress = true; - close = new ArrayList<>(connections); + close = new ArrayList<>(connections.size()); + for (ConnectionRecord rec : connections) close.add(rec.connection); connections.clear(); } if (LOG.isLoggable(INFO)) { @@ -62,7 +72,9 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { if (keyAgreementInProgress) { LOG.info("Can't open contact connection during key agreement"); return false; - } else if (connections.size() >= connectionLimit) { + } + considerRaisingConnectionLimit(clock.currentTimeMillis()); + if (connections.size() >= connectionLimit) { LOG.info("Can't open contact connection due to limit"); return false; } else { @@ -79,12 +91,16 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { if (keyAgreementInProgress) { LOG.info("Refusing contact connection during key agreement"); accept = false; - } else if (connections.size() > connectionLimit) { - LOG.info("Refusing contact connection due to limit"); - accept = false; } else { - LOG.info("Accepting contact connection"); - connections.add(conn); + long now = clock.currentTimeMillis(); + considerRaisingConnectionLimit(now); + if (connections.size() > connectionLimit) { + LOG.info("Refusing contact connection due to limit"); + accept = false; + } else { + LOG.info("Accepting contact connection"); + connections.add(new ConnectionRecord(conn, now)); + } } } if (!accept) tryToClose(conn); @@ -95,7 +111,8 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { public void keyAgreementConnectionOpened(DuplexTransportConnection conn) { synchronized (lock) { LOG.info("Accepting key agreement connection"); - connections.add(conn); + connections.add( + new ConnectionRecord(conn, clock.currentTimeMillis())); } } @@ -111,7 +128,13 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { @Override public void connectionClosed(DuplexTransportConnection conn) { synchronized (lock) { - connections.remove(conn); + Iterator it = connections.iterator(); + while (it.hasNext()) { + if (it.next().connection == conn) { + it.remove(); + break; + } + } if (LOG.isLoggable(INFO)) LOG.info("Connection closed, " + connections.size() + " open"); } @@ -124,4 +147,32 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter { LOG.info("All connections closed"); } } + + @GuardedBy("lock") + private void considerRaisingConnectionLimit(long now) { + int stable = 0; + for (ConnectionRecord rec : connections) { + if (now - rec.timeOpened >= STABILITY_PERIOD_MS) stable++; + } + if (stable >= connectionLimit) { + LOG.info("Raising connection limit"); + connectionLimit++; + } + if (LOG.isLoggable(INFO)) { + LOG.info(stable + " connections are stable, limit is " + + connectionLimit); + } + } + + private static final class ConnectionRecord { + + private final DuplexTransportConnection connection; + private final long timeOpened; + + private ConnectionRecord(DuplexTransportConnection connection, + long timeOpened) { + this.connection = connection; + this.timeOpened = timeOpened; + } + } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImplTest.java index c9eb3a408..c87573827 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImplTest.java @@ -3,15 +3,18 @@ package org.briarproject.bramble.plugin.bluetooth; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.test.BrambleMockTestCase; import org.jmock.Expectations; import org.junit.Test; +import static org.briarproject.bramble.plugin.bluetooth.BluetoothConnectionLimiter.STABILITY_PERIOD_MS; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class BluetoothConnectionLimiterImplTest extends BrambleMockTestCase { + private final Clock clock = context.mock(Clock.class); private final DuplexTransportConnection conn = context.mock(DuplexTransportConnection.class); private final TransportConnectionReader reader = @@ -19,29 +22,104 @@ public class BluetoothConnectionLimiterImplTest extends BrambleMockTestCase { private final TransportConnectionWriter writer = context.mock(TransportConnectionWriter.class); + private final long now = System.currentTimeMillis(); + @Test public void testLimiterAllowsOneOutgoingConnection() { BluetoothConnectionLimiter limiter = - new BluetoothConnectionLimiterImpl(); + new BluetoothConnectionLimiterImpl(clock); + + expectGetCurrentTime(now); assertTrue(limiter.canOpenContactConnection()); + + expectGetCurrentTime(now); assertTrue(limiter.contactConnectionOpened(conn)); + + expectGetCurrentTime(now); assertFalse(limiter.canOpenContactConnection()); } @Test public void testLimiterAllowsSecondIncomingConnection() throws Exception { BluetoothConnectionLimiter limiter = - new BluetoothConnectionLimiterImpl(); + new BluetoothConnectionLimiterImpl(clock); + + expectGetCurrentTime(now); assertTrue(limiter.canOpenContactConnection()); + + expectGetCurrentTime(now); assertTrue(limiter.contactConnectionOpened(conn)); + + expectGetCurrentTime(now); assertFalse(limiter.canOpenContactConnection()); + // The limiter allows a second incoming connection + expectGetCurrentTime(now); assertTrue(limiter.contactConnectionOpened(conn)); - // The limiter closes any further incoming connections + + // The limiter does not allow a third incoming connection + expectGetCurrentTime(now); expectCloseConnection(); assertFalse(limiter.contactConnectionOpened(conn)); } + @Test + public void testLimiterAllowsSecondOutgoingConnectionWhenFirstIsStable() + throws Exception { + BluetoothConnectionLimiter limiter = + new BluetoothConnectionLimiterImpl(clock); + + expectGetCurrentTime(now); + assertTrue(limiter.canOpenContactConnection()); + + expectGetCurrentTime(now); + assertTrue(limiter.contactConnectionOpened(conn)); + + // The first connection is not yet stable + expectGetCurrentTime(now + STABILITY_PERIOD_MS - 1); + assertFalse(limiter.canOpenContactConnection()); + + // The first connection is stable, so the limit is raised + expectGetCurrentTime(now + STABILITY_PERIOD_MS); + assertTrue(limiter.canOpenContactConnection()); + } + + @Test + public void testLimiterAllowsThirdIncomingConnectionWhenFirstTwoAreStable() + throws Exception { + BluetoothConnectionLimiter limiter = + new BluetoothConnectionLimiterImpl(clock); + + expectGetCurrentTime(now); + assertTrue(limiter.canOpenContactConnection()); + + expectGetCurrentTime(now); + assertTrue(limiter.contactConnectionOpened(conn)); + + expectGetCurrentTime(now); + assertFalse(limiter.canOpenContactConnection()); + + // The limiter allows a second incoming connection + expectGetCurrentTime(now); + assertTrue(limiter.contactConnectionOpened(conn)); + + // The limiter does not allow a third incoming connection + expectGetCurrentTime(now + STABILITY_PERIOD_MS - 1); + expectCloseConnection(); + assertFalse(limiter.contactConnectionOpened(conn)); + + // The first two connections are stable, so the limit is raised + expectGetCurrentTime(now + STABILITY_PERIOD_MS); + assertTrue(limiter.contactConnectionOpened(conn)); + } + + private void expectGetCurrentTime(long now) { + context.checking(new Expectations() {{ + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + }}); + } + private void expectCloseConnection() throws Exception { context.checking(new Expectations() {{ oneOf(conn).getReader(); diff --git a/bramble-java/src/main/java/org/briarproject/bramble/plugin/DesktopPluginModule.java b/bramble-java/src/main/java/org/briarproject/bramble/plugin/DesktopPluginModule.java index c00bd99d7..185d308a0 100644 --- a/bramble-java/src/main/java/org/briarproject/bramble/plugin/DesktopPluginModule.java +++ b/bramble-java/src/main/java/org/briarproject/bramble/plugin/DesktopPluginModule.java @@ -10,6 +10,7 @@ import org.briarproject.bramble.api.plugin.PluginConfig; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; import org.briarproject.bramble.api.plugin.simplex.SimplexPluginFactory; import org.briarproject.bramble.api.reliability.ReliabilityLayerFactory; +import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.plugin.bluetooth.JavaBluetoothPluginFactory; import org.briarproject.bramble.plugin.modem.ModemPluginFactory; import org.briarproject.bramble.plugin.tcp.LanTcpPluginFactory; @@ -32,10 +33,11 @@ public class DesktopPluginModule extends PluginModule { PluginConfig getPluginConfig(@IoExecutor Executor ioExecutor, SecureRandom random, BackoffFactory backoffFactory, ReliabilityLayerFactory reliabilityFactory, - ShutdownManager shutdownManager, EventBus eventBus, + ShutdownManager shutdownManager, EventBus eventBus, Clock clock, TimeoutMonitor timeoutMonitor) { DuplexPluginFactory bluetooth = new JavaBluetoothPluginFactory( - ioExecutor, random, eventBus, timeoutMonitor, backoffFactory); + ioExecutor, random, eventBus, clock, timeoutMonitor, + backoffFactory); DuplexPluginFactory modem = new ModemPluginFactory(ioExecutor, reliabilityFactory); DuplexPluginFactory lan = new LanTcpPluginFactory(ioExecutor, diff --git a/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java b/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java index 59fc25c86..10ed6db1e 100644 --- a/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java +++ b/bramble-java/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothPluginFactory.java @@ -9,6 +9,7 @@ import org.briarproject.bramble.api.plugin.PluginCallback; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexPlugin; import org.briarproject.bramble.api.plugin.duplex.DuplexPluginFactory; +import org.briarproject.bramble.api.system.Clock; import java.security.SecureRandom; import java.util.concurrent.Executor; @@ -30,15 +31,17 @@ public class JavaBluetoothPluginFactory implements DuplexPluginFactory { private final Executor ioExecutor; private final SecureRandom secureRandom; private final EventBus eventBus; + private final Clock clock; private final TimeoutMonitor timeoutMonitor; private final BackoffFactory backoffFactory; public JavaBluetoothPluginFactory(Executor ioExecutor, - SecureRandom secureRandom, EventBus eventBus, + SecureRandom secureRandom, EventBus eventBus, Clock clock, TimeoutMonitor timeoutMonitor, BackoffFactory backoffFactory) { this.ioExecutor = ioExecutor; this.secureRandom = secureRandom; this.eventBus = eventBus; + this.clock = clock; this.timeoutMonitor = timeoutMonitor; this.backoffFactory = backoffFactory; } @@ -56,7 +59,7 @@ public class JavaBluetoothPluginFactory implements DuplexPluginFactory { @Override public DuplexPlugin createPlugin(PluginCallback callback) { BluetoothConnectionLimiter connectionLimiter = - new BluetoothConnectionLimiterImpl(); + new BluetoothConnectionLimiterImpl(clock); Backoff backoff = backoffFactory.createBackoff(MIN_POLLING_INTERVAL, MAX_POLLING_INTERVAL, BACKOFF_BASE); JavaBluetoothPlugin plugin = new JavaBluetoothPlugin(connectionLimiter,