diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index 15373f213..a9689b5a8 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -40,7 +40,7 @@ class AndroidBluetoothTransportConnection try { socket.close(); } finally { - connectionManager.connectionClosed(); + connectionManager.connectionClosed(this); } } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManager.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManager.java index c6e722396..7c8726230 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManager.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManager.java @@ -1,26 +1,36 @@ package org.briarproject.bramble.plugin.bluetooth; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +@NotNullByDefault interface BluetoothConnectionManager { /** * Returns true if a contact connection can be opened without exceeding - * the connection limit. + * the connection limit. This method does not need to be called for key + * exchange connections. */ boolean canOpenConnection(); /** - * Increments the number of open connections and returns true if the new - * connection can be kept open without exceeding the connection limit. + * Passes a newly opened connection to the manager. The manager may close + * the new connection or another connection to stay within the connection + * limit. + *

+ * Returns false if the manager has closed the new connection (this will + * never be the case for key exchange connections). */ - boolean connectionOpened(); + boolean connectionOpened(DuplexTransportConnection conn, + boolean isForKeyExchange); /** - * Decrements the number of open connections. + * Informs the manager that the given connection has been closed. */ - void connectionClosed(); + void connectionClosed(DuplexTransportConnection conn); /** - * Resets the number of open connections. + * Informs the manager that all connections have been closed. */ void allConnectionsClosed(); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManagerImpl.java index 7d40e7831..e1dd7d675 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothConnectionManagerImpl.java @@ -1,10 +1,19 @@ package org.briarproject.bramble.plugin.bluetooth; -import java.util.concurrent.atomic.AtomicInteger; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; + +import java.io.IOException; +import java.util.LinkedList; import java.util.logging.Logger; -import static java.util.logging.Level.INFO; +import javax.annotation.concurrent.ThreadSafe; +import static java.util.logging.Level.INFO; +import static java.util.logging.Level.WARNING; + +@NotNullByDefault +@ThreadSafe class BluetoothConnectionManagerImpl implements BluetoothConnectionManager { private static final int MAX_OPEN_CONNECTIONS = 5; @@ -12,34 +21,69 @@ class BluetoothConnectionManagerImpl implements BluetoothConnectionManager { private static final Logger LOG = Logger.getLogger(BluetoothConnectionManagerImpl.class.getName()); - private final AtomicInteger openConnections = new AtomicInteger(0); + private final Object lock = new Object(); + private final LinkedList connections = + new LinkedList<>(); // Locking: lock @Override public boolean canOpenConnection() { - int open = openConnections.get(); - if (LOG.isLoggable(INFO)) - LOG.info(open + " open connections"); - return open < MAX_OPEN_CONNECTIONS; + synchronized (lock) { + int open = connections.size(); + if (LOG.isLoggable(INFO)) LOG.info(open + " open connections"); + return open < MAX_OPEN_CONNECTIONS; + } } @Override - public boolean connectionOpened() { - int open = openConnections.incrementAndGet(); - if (LOG.isLoggable(INFO)) - LOG.info("Connection opened, " + open + " open"); - return open <= MAX_OPEN_CONNECTIONS; + public boolean connectionOpened(DuplexTransportConnection conn, + boolean isForKeyExchange) { + DuplexTransportConnection close = null; + synchronized (lock) { + int open = connections.size(); + boolean accept = isForKeyExchange || open < MAX_OPEN_CONNECTIONS; + if (accept) { + if (LOG.isLoggable(INFO)) + LOG.info("Accepting connection, " + (open + 1) + " open"); + connections.add(conn); + if (open == MAX_OPEN_CONNECTIONS) { + LOG.info("Closing old connection to stay within limit"); + close = connections.poll(); + } + } else { + if (LOG.isLoggable(INFO)) + LOG.info("Refusing connection, " + open + " open"); + close = conn; + } + } + if (close != null) tryToClose(close); + return close != conn; + } + + private void tryToClose(DuplexTransportConnection conn) { + try { + conn.getWriter().dispose(false); + conn.getReader().dispose(false, false); + } catch (IOException e) { + if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } } @Override - public void connectionClosed() { - int open = openConnections.decrementAndGet(); - if (LOG.isLoggable(INFO)) - LOG.info("Connection closed, " + open + " open"); + public void connectionClosed(DuplexTransportConnection conn) { + synchronized (lock) { + connections.remove(conn); + if (LOG.isLoggable(INFO)) { + int open = connections.size(); + LOG.info("Connection closed, " + open + " open"); + } + } } @Override public void allConnectionsClosed() { - LOG.info("All connections closed"); - openConnections.set(0); + synchronized (lock) { + connections.clear(); + LOG.info("All connections closed"); + } } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java index 68c64efc3..594bcff94 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/bluetooth/BluetoothPlugin.java @@ -219,25 +219,12 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { return; } backoff.reset(); - if (connectionManager.connectionOpened()) { + if (connectionManager.connectionOpened(conn, false)) callback.incomingConnectionCreated(conn); - } else { - LOG.info("Closing incoming connection"); - tryToCloseUnusedConnection(conn); - } if (!running) return; } } - private void tryToCloseUnusedConnection(DuplexTransportConnection conn) { - try { - conn.getWriter().dispose(false); - conn.getReader().dispose(false, false); - } catch (IOException e) { - if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - } - } - @Override public void stop() { running = false; @@ -284,12 +271,8 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { DuplexTransportConnection conn = connect(address, uuid); if (conn != null) { backoff.reset(); - if (connectionManager.connectionOpened()) { + if (connectionManager.connectionOpened(conn, false)) callback.outgoingConnectionCreated(c, conn); - } else { - LOG.info("Closing outgoing connection"); - tryToCloseUnusedConnection(conn); - } } }); } @@ -341,13 +324,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { DuplexTransportConnection conn = connect(address, uuid); if (conn == null) return null; // TODO: Why don't we reset the backoff here? - if (connectionManager.connectionOpened()) { - return conn; - } else { - LOG.info("Closing outgoing connection"); - tryToCloseUnusedConnection(conn); - return null; - } + return connectionManager.connectionOpened(conn, false) ? conn : null; } @Override @@ -399,7 +376,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { LOG.info("Connecting to key agreement UUID " + uuid); DuplexTransportConnection conn = connect(address, uuid); // The connection limit doesn't apply to key agreement - if (conn != null) connectionManager.connectionOpened(); + if (conn != null) connectionManager.connectionOpened(conn, true); return conn; } @@ -456,7 +433,7 @@ abstract class BluetoothPlugin implements DuplexPlugin, EventListener { if (LOG.isLoggable(INFO)) LOG.info(ID.getString() + ": Incoming connection"); // The connection limit doesn't apply to key agreement - connectionManager.connectionOpened(); + connectionManager.connectionOpened(conn, true); return new KeyAgreementConnection(conn, ID); }; } diff --git a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java index 06b894f12..c94d7524a 100644 --- a/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java +++ b/bramble-j2se/src/main/java/org/briarproject/bramble/plugin/bluetooth/JavaBluetoothTransportConnection.java @@ -40,7 +40,7 @@ class JavaBluetoothTransportConnection try { stream.close(); } finally { - connectionManager.connectionClosed(); + connectionManager.connectionClosed(this); } } }