Back off between attempts to raise connection limit.

bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiter.java

@@ -19,7 +19,14 @@ interface BluetoothConnectionLimiter {
 	 * This is longer than {@link #STABILITY_PERIOD_MS} so we don't start
 	 * another attempt before knowing the outcome of the last one.
 	 */
-	long MIN_ATTEMPT_INTERVAL_MS = MINUTES.toMillis(5);
+	long MIN_ATTEMPT_INTERVAL_MS = MINUTES.toMillis(2);
+
+	/**
+	 * The maximum exponent to use when backing off between attempts to raise
+	 * the connection limit. The maximum interval between attempts is
+	 * MIN_ATTEMPT_INTERVAL_MS * (1L << MAX_ATTEMPT_BACKOFF) =~ 34 hours.
+	 */
+	int MAX_ATTEMPT_BACKOFF = 10;
 
 	/**
 	 * Informs the limiter that key agreement has started.

bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImpl.java

@@ -16,6 +16,7 @@ import javax.annotation.concurrent.GuardedBy;
 import javax.annotation.concurrent.ThreadSafe;
 import javax.inject.Inject;
 
+import static java.lang.Math.min;
 import static java.util.logging.Level.INFO;
 import static java.util.logging.Level.WARNING;
 import static java.util.logging.Logger.getLogger;
@@ -38,9 +39,10 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter {
 	@GuardedBy("lock")
 	private boolean keyAgreementInProgress = false;
 	@GuardedBy("lock")
-	private int connectionLimit = 1;
+	private int connectionLimit = 1, attemptBackoff = 0;
 	@GuardedBy("lock")
-	private long timeOfLastAttempt = 0;
+	private long timeOfLastAttempt = 0,
+			attemptInterval = MIN_ATTEMPT_INTERVAL_MS;
 
 	@Inject
 	BluetoothConnectionLimiterImpl(EventBus eventBus, Clock clock) {
@@ -123,10 +125,14 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter {
 	public void connectionClosed(DuplexTransportConnection conn,
 			boolean exception) {
 		synchronized (lock) {
+			int numConnections = connections.size();
 			Iterator<ConnectionRecord> it = connections.iterator();
 			while (it.hasNext()) {
 				if (it.next().connection == conn) {
 					it.remove();
+					if (exception && numConnections > connectionLimit) {
+						connectionFailedAboveLimit();
+					}
 					break;
 				}
 			}
@@ -152,7 +158,7 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter {
 		} else if (connections.size() < connectionLimit) {
 			LOG.info("Allowing contact connection, below limit");
 			return true;
-		} else if (canAttemptToRaiseLimit(now)) {
+		} else if (now - timeOfLastAttempt >= attemptInterval) {
 			LOG.info("Allowing contact connection, at limit");
 			return true;
 		} else {
@@ -170,6 +176,8 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter {
 		if (stable > connectionLimit) {
 			LOG.info("Raising connection limit");
 			connectionLimit++;
+			attemptBackoff = 0;
+			attemptInterval = MIN_ATTEMPT_INTERVAL_MS;
 		}
 		if (LOG.isLoggable(INFO)) {
 			LOG.info(stable + " connections are stable, limit is "
@@ -178,8 +186,13 @@ class BluetoothConnectionLimiterImpl implements BluetoothConnectionLimiter {
 	}
 
 	@GuardedBy("lock")
-	private boolean canAttemptToRaiseLimit(long now) {
+	private void connectionFailedAboveLimit() {
-		return now - timeOfLastAttempt >= MIN_ATTEMPT_INTERVAL_MS;
+		long now = clock.currentTimeMillis();
+		if (now - timeOfLastAttempt < STABILITY_PERIOD_MS) {
+			LOG.info("Connection failed above limit, increasing interval");
+			attemptBackoff = min(attemptBackoff + 1, MAX_ATTEMPT_BACKOFF);
+			attemptInterval = MIN_ATTEMPT_INTERVAL_MS * (1L << attemptBackoff);
+		}
 	}
 
 	private static final class ConnectionRecord {

bramble-core/src/test/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionLimiterImplTest.java

@@ -4,12 +4,17 @@ import org.briarproject.bramble.api.event.EventBus;
 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.sync.event.CloseSyncConnectionsEvent;
 import org.briarproject.bramble.api.system.Clock;
 import org.briarproject.bramble.test.BrambleMockTestCase;
+import org.briarproject.bramble.test.SettableClock;
 import org.jmock.Expectations;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.concurrent.atomic.AtomicLong;
+
+import static org.briarproject.bramble.plugin.bluetooth.BluetoothConnectionLimiter.MIN_ATTEMPT_INTERVAL_MS;
 import static org.briarproject.bramble.plugin.bluetooth.BluetoothConnectionLimiter.STABILITY_PERIOD_MS;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -17,7 +22,6 @@ import static org.junit.Assert.assertTrue;
 public class BluetoothConnectionLimiterImplTest extends BrambleMockTestCase {
 
 	private final EventBus eventBus = context.mock(EventBus.class);
-	private final Clock clock = context.mock(Clock.class);
 
 	private final DuplexTransportConnection conn1 =
 			context.mock(DuplexTransportConnection.class, "conn1");
@@ -32,117 +36,158 @@ public class BluetoothConnectionLimiterImplTest extends BrambleMockTestCase {
 
 	private final long now = System.currentTimeMillis();
 
+	private AtomicLong time;
 	private BluetoothConnectionLimiter limiter;
 
 	@Before
 	public void setUp() {
+		time = new AtomicLong(now);
+		Clock clock = new SettableClock(time);
 		limiter = new BluetoothConnectionLimiterImpl(eventBus, clock);
 	}
 
+	@Test
+	public void testLimiterDoesNotAllowContactConnectionsDuringKeyAgreement() {
+		assertTrue(limiter.canOpenContactConnection());
+
+		expectCloseSyncConnectionsEvent();
+		limiter.keyAgreementStarted();
+
+		assertFalse(limiter.canOpenContactConnection());
+
+		limiter.keyAgreementEnded();
+
+		assertTrue(limiter.canOpenContactConnection());
+	}
+
 	@Test
 	public void testLimiterAllowsAttemptToRaiseLimitAtStartup()
 			throws Exception {
-		// First outgoing connection is allowed - we're below the limit
+		// First outgoing connection is allowed - we're below the limit of 1
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn1));
 
 		// Second outgoing connection is allowed - it's time to try raising
-		// the limit
+		// the limit to 2
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn2));
 
-		// Third outgoing connection is not allowed - we're above the limit
+		// Third outgoing connection is not allowed - we're above the limit of 1
-		expectGetCurrentTime(now);
 		assertFalse(limiter.canOpenContactConnection());
 
-		// Third incoming connection is not allowed - we're above the limit
+		// Third incoming connection is not allowed - we're above the limit of 1
-		expectGetCurrentTime(now);
 		expectCloseConnection(conn3);
 		assertFalse(limiter.contactConnectionOpened(conn3));
 	}
 
 	@Test
 	public void testLimiterAllowsThirdConnectionAfterFirstTwoAreClosed() {
-		// First outgoing connection is allowed - we're below the limit
+		// First outgoing connection is allowed - we're below the limit of 1
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn1));
 
 		// Second outgoing connection is allowed - it's time to try raising
-		// the limit
+		// the limit to 2
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn2));
 
-		// Third outgoing connection is not allowed - we're above the limit
+		// Third outgoing connection is not allowed - we're above the limit of 1
-		expectGetCurrentTime(now);
 		assertFalse(limiter.canOpenContactConnection());
 
 		// Close the first connection
 		limiter.connectionClosed(conn1, false);
 
-		// Third outgoing connection is not allowed - we're at the limit
+		// Third outgoing connection is not allowed - we're at the limit of 1
-		expectGetCurrentTime(now);
 		assertFalse(limiter.canOpenContactConnection());
 
 		// Close the second connection
 		limiter.connectionClosed(conn2, false);
 
-		// Third outgoing connection is allowed - we're below the limit
+		// Third outgoing connection is allowed - we're below the limit of 1
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn3));
 	}
 
 	@Test
 	public void testLimiterRaisesLimitWhenConnectionsAreStable() {
-		// First outgoing connection is allowed - we're below the limit
+		// First outgoing connection is allowed - we're below the limit of 1
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn1));
 
 		// Second outgoing connection is allowed - it's time to try raising
-		// the limit
+		// the limit to 2
-		expectGetCurrentTime(now);
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now);
 		assertTrue(limiter.contactConnectionOpened(conn2));
 
-		// Third outgoing connection is not allowed - we're above the limit
+		// Third outgoing connection is not allowed - we're above the limit of 1
-		expectGetCurrentTime(now);
 		assertFalse(limiter.canOpenContactConnection());
 
-		// Third outgoing connection is still not allowed - first two are
+		// Time passes
-		// stable but we're still at the limit
+		time.set(now + STABILITY_PERIOD_MS);
-		expectGetCurrentTime(now + STABILITY_PERIOD_MS);
+
+		// Third outgoing connection is still not allowed - first two are now
+		// stable so limit is raised to 2, but we're already at the new limit
 		assertFalse(limiter.canOpenContactConnection());
 
-		// Close the first connection
+		// Time passes
-		limiter.connectionClosed(conn1, false);
+		time.set(now + MIN_ATTEMPT_INTERVAL_MS);
 
-		// Third outgoing connection is allowed - we're below the new limit
+		// Third outgoing connection is allowed - it's time to try raising
-		expectGetCurrentTime(now + STABILITY_PERIOD_MS);
+		// the limit to 3
 		assertTrue(limiter.canOpenContactConnection());
-		expectGetCurrentTime(now + STABILITY_PERIOD_MS);
 		assertTrue(limiter.contactConnectionOpened(conn3));
 
-		// Fourth outgoing connection is not allowed
+		// Fourth outgoing connection is not allowed - we're above the limit
-		expectGetCurrentTime(now + STABILITY_PERIOD_MS);
+		// of 2
 		assertFalse(limiter.canOpenContactConnection());
 	}
 
-	private void expectGetCurrentTime(long now) {
+	@Test
+	public void testLimiterIncreasesIntervalWhenConnectionFailsAboveLimit() {
+		// First outgoing connection is allowed - we're below the limit of 1
+		assertTrue(limiter.canOpenContactConnection());
+		assertTrue(limiter.contactConnectionOpened(conn1));
+
+		// Time passes
+		time.set(now + 1);
+
+		// Second outgoing connection is allowed - it's time to try raising
+		// the limit to 2
+		assertTrue(limiter.canOpenContactConnection());
+		assertTrue(limiter.contactConnectionOpened(conn2));
+
+		// Time passes - the first connection is stable, the second isn't
+		time.set(now + STABILITY_PERIOD_MS);
+
+		// First connection fails. The second connection isn't stable yet, so
+		// the limiter considers this a failed attempt and doubles the interval
+		// between attempts
+		limiter.connectionClosed(conn1, true);
+
+		// Third outgoing connection is not allowed - we're still at the limit
+		// of 1
+		assertFalse(limiter.canOpenContactConnection());
+
+		// Time passes - nearly time for the second attempt
+		time.set(now + MIN_ATTEMPT_INTERVAL_MS * 2);
+
+		// Third outgoing connection is not allowed - we're still at the limit
+		// of 1
+		assertFalse(limiter.canOpenContactConnection());
+
+		// Time passes - now it's time for the second attempt
+		time.set(now + 1 + MIN_ATTEMPT_INTERVAL_MS * 2);
+
+		// Third outgoing connection is allowed - it's time to try raising the
+		// limit to 2 again
+		assertTrue(limiter.canOpenContactConnection());
+		assertTrue(limiter.contactConnectionOpened(conn3));
+	}
+
+	private void expectCloseSyncConnectionsEvent() {
 		context.checking(new Expectations() {{
-			oneOf(clock).currentTimeMillis();
+			oneOf(eventBus).broadcast(with(any(
-			will(returnValue(now));
+					CloseSyncConnectionsEvent.class)));
 		}});
 	}