diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java b/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java index 1b552101e..db5968902 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/connection/ConnectionRegistry.java @@ -47,7 +47,7 @@ public interface ConnectionRegistry { * the contact. */ void unregisterConnection(ContactId c, TransportId t, - InterruptibleConnection conn, boolean incoming); + InterruptibleConnection conn, boolean incoming, boolean exception); /** * Sets the {@link Priority priority} of a connection that was previously diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/event/ConnectionClosedEvent.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/event/ConnectionClosedEvent.java index 05c4cbdd7..6d5e5293b 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/event/ConnectionClosedEvent.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/event/ConnectionClosedEvent.java @@ -13,13 +13,14 @@ public class ConnectionClosedEvent extends Event { private final ContactId contactId; private final TransportId transportId; - private final boolean incoming; + private final boolean incoming, exception; public ConnectionClosedEvent(ContactId contactId, TransportId transportId, - boolean incoming) { + boolean incoming, boolean exception) { this.contactId = contactId; this.transportId = transportId; this.incoming = incoming; + this.exception = exception; } public ContactId getContactId() { @@ -33,4 +34,8 @@ public class ConnectionClosedEvent extends Event { public boolean isIncoming() { return incoming; } + + public boolean isException() { + return exception; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java index d735a7c2f..3b3d11e8b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java @@ -136,7 +136,7 @@ class ConnectionRegistryImpl implements ConnectionRegistry { @Override public void unregisterConnection(ContactId c, TransportId t, - InterruptibleConnection conn, boolean incoming) { + InterruptibleConnection conn, boolean incoming, boolean exception) { if (LOG.isLoggable(INFO)) { if (incoming) LOG.info("Incoming connection unregistered: " + t); else LOG.info("Outgoing connection unregistered: " + t); @@ -148,7 +148,8 @@ class ConnectionRegistryImpl implements ConnectionRegistry { throw new IllegalArgumentException(); lastConnection = recs.isEmpty(); } - eventBus.broadcast(new ConnectionClosedEvent(c, t, incoming)); + eventBus.broadcast( + new ConnectionClosedEvent(c, t, incoming, exception)); if (lastConnection) { LOG.info("Contact disconnected"); eventBus.broadcast(new ContactDisconnectedEvent(c)); @@ -178,8 +179,7 @@ class ConnectionRegistryImpl implements ConnectionRegistry { @Override public Collection getConnectedOrBetterContacts(TransportId t) { synchronized (lock) { - List better = betterTransports.get(t); - if (better == null) better = emptyList(); + List better = getBetterTransports(t); List contactIds = new ArrayList<>(); for (Entry> e : contactConnections.entrySet()) { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java index f1b53e768..f1fa9fe2c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/IncomingDuplexSyncConnection.java @@ -74,12 +74,13 @@ class IncomingDuplexSyncConnection extends DuplexSyncConnection createIncomingSession(ctx, reader, handler).run(); reader.dispose(false, true); interruptOutgoingSession(); + connectionRegistry.unregisterConnection(contactId, transportId, + this, true, false); } catch (DbException | IOException e) { logException(LOG, WARNING, e); onReadError(true); - } finally { connectionRegistry.unregisterConnection(contactId, transportId, - this, true); + this, true, true); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java b/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java index a4d237071..958f651dc 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/connection/OutgoingDuplexSyncConnection.java @@ -118,12 +118,13 @@ class OutgoingDuplexSyncConnection extends DuplexSyncConnection createIncomingSession(ctx, reader, handler).run(); reader.dispose(false, true); interruptOutgoingSession(); + connectionRegistry.unregisterConnection(contactId, transportId, + this, false, false); } catch (DbException | IOException e) { logException(LOG, WARNING, e); onReadError(); - } finally { connectionRegistry.unregisterConnection(contactId, transportId, - this, false); + this, false, true); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java index a0b72dc55..1aeb66172 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/PollerImpl.java @@ -98,8 +98,8 @@ class PollerImpl implements Poller, EventListener { ConnectionClosedEvent c = (ConnectionClosedEvent) e; // Reschedule polling, the polling interval may have decreased reschedule(c.getTransportId()); - if (!c.isIncoming()) { - // Connect to the disconnected contact + // If an outgoing connection failed, try to reconnect + if (!c.isIncoming() && c.isException()) { connectToContact(c.getContactId(), c.getTransportId()); } } else if (e instanceof ConnectionOpenedEvent) { diff --git a/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java index 1a1fbc6b1..3341efdb9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java @@ -108,7 +108,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionClosedEvent.class))); }}); - c.unregisterConnection(contactId1, transportId1, conn1, true); + c.unregisterConnection(contactId1, transportId1, conn1, true, false); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -123,7 +123,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { oneOf(eventBus).broadcast(with(any( ContactDisconnectedEvent.class))); }}); - c.unregisterConnection(contactId1, transportId1, conn2, true); + c.unregisterConnection(contactId1, transportId1, conn2, true, false); context.assertIsSatisfied(); assertEquals(emptyList(), c.getConnectedContacts(transportId1)); @@ -131,7 +131,8 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { // Try to unregister the connection again - exception should be thrown try { - c.unregisterConnection(contactId1, transportId1, conn2, true); + c.unregisterConnection(contactId1, transportId1, conn2, + true, false); fail(); } catch (IllegalArgumentException expected) { // Expected @@ -332,7 +333,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionClosedEvent.class))); }}); - c.unregisterConnection(contactId1, transportId2, conn2, true); + c.unregisterConnection(contactId1, transportId2, conn2, true, false); context.assertIsSatisfied(); assertEquals(singletonList(contactId1), @@ -429,7 +430,7 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ oneOf(eventBus).broadcast(with(any(ConnectionClosedEvent.class))); }}); - c.unregisterConnection(contactId1, transportId1, conn1, true); + c.unregisterConnection(contactId1, transportId1, conn1, true, false); context.assertIsSatisfied(); // The contact is not connected via transport 1 but is connected via a diff --git a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java index 9d56b2ef4..cf29a7a0e 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/plugin/PollerImplTest.java @@ -157,7 +157,21 @@ public class PollerImplTest extends BrambleMockTestCase { } @Test - public void testRescheduleAndReconnectOnConnectionClosed() + public void testRescheduleOnOutgoingConnectionClosed() { + DuplexPlugin plugin = context.mock(DuplexPlugin.class); + + context.checking(new Expectations() {{ + allowing(plugin).getId(); + will(returnValue(transportId)); + }}); + expectReschedule(plugin); + + poller.eventOccurred(new ConnectionClosedEvent(contactId, transportId, + false, false)); + } + + @Test + public void testRescheduleAndReconnectOnOutgoingConnectionFailed() throws Exception { DuplexPlugin plugin = context.mock(DuplexPlugin.class); DuplexTransportConnection duplexConnection = @@ -166,45 +180,40 @@ public class PollerImplTest extends BrambleMockTestCase { context.checking(new Expectations() {{ allowing(plugin).getId(); will(returnValue(transportId)); - // reschedule() - // Get the plugin - oneOf(pluginManager).getPlugin(transportId); - will(returnValue(plugin)); - // The plugin supports polling - oneOf(plugin).shouldPoll(); - will(returnValue(true)); - // Get the plugin - oneOf(pluginManager).getPlugin(transportId); - will(returnValue(plugin)); - // The plugin supports polling - oneOf(plugin).shouldPoll(); - will(returnValue(true)); - // Schedule the next poll - oneOf(plugin).getPollingInterval(); - will(returnValue(pollingInterval)); - oneOf(clock).currentTimeMillis(); - will(returnValue(now)); - oneOf(scheduler).schedule(with(any(Runnable.class)), - with((long) pollingInterval), with(MILLISECONDS)); - will(returnValue(future)); - // connectToContact() - // Check whether the contact is already connected - oneOf(connectionRegistry).isConnected(contactId, transportId); - will(returnValue(false)); - // Get the transport properties - oneOf(transportPropertyManager).getRemoteProperties(contactId, - transportId); - will(returnValue(properties)); - // Connect to the contact - oneOf(plugin).createConnection(properties); - will(returnValue(duplexConnection)); - // Pass the connection to the connection manager - oneOf(connectionManager).manageOutgoingConnection(contactId, - transportId, duplexConnection); }}); + expectReschedule(plugin); + expectReconnect(plugin, duplexConnection); poller.eventOccurred(new ConnectionClosedEvent(contactId, transportId, - false)); + false, true)); + } + + @Test + public void testRescheduleOnIncomingConnectionClosed() { + DuplexPlugin plugin = context.mock(DuplexPlugin.class); + + context.checking(new Expectations() {{ + allowing(plugin).getId(); + will(returnValue(transportId)); + }}); + expectReschedule(plugin); + + poller.eventOccurred(new ConnectionClosedEvent(contactId, transportId, + true, false)); + } + + @Test + public void testRescheduleOnIncomingConnectionFailed() { + DuplexPlugin plugin = context.mock(DuplexPlugin.class); + + context.checking(new Expectations() {{ + allowing(plugin).getId(); + will(returnValue(transportId)); + }}); + expectReschedule(plugin); + + poller.eventOccurred(new ConnectionClosedEvent(contactId, transportId, + true, false)); } @Test @@ -431,4 +440,48 @@ public class PollerImplTest extends BrambleMockTestCase { poller.eventOccurred(new TransportEnabledEvent(transportId)); poller.eventOccurred(new TransportDisabledEvent(transportId)); } + + private void expectReschedule(Plugin plugin) { + context.checking(new Expectations() {{ + // Get the plugin + oneOf(pluginManager).getPlugin(transportId); + will(returnValue(plugin)); + // The plugin supports polling + oneOf(plugin).shouldPoll(); + will(returnValue(true)); + // Schedule the next poll + oneOf(plugin).getPollingInterval(); + will(returnValue(pollingInterval)); + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + oneOf(scheduler).schedule(with(any(Runnable.class)), + with((long) pollingInterval), with(MILLISECONDS)); + will(returnValue(future)); + }}); + } + + private void expectReconnect(DuplexPlugin plugin, + DuplexTransportConnection duplexConnection) throws Exception { + context.checking(new Expectations() {{ + // Get the plugin + oneOf(pluginManager).getPlugin(transportId); + will(returnValue(plugin)); + // The plugin supports polling + oneOf(plugin).shouldPoll(); + will(returnValue(true)); + // Check whether the contact is already connected + oneOf(connectionRegistry).isConnected(contactId, transportId); + will(returnValue(false)); + // Get the transport properties + oneOf(transportPropertyManager).getRemoteProperties(contactId, + transportId); + will(returnValue(properties)); + // Connect to the contact + oneOf(plugin).createConnection(properties); + will(returnValue(duplexConnection)); + // Pass the connection to the connection manager + oneOf(connectionManager).manageOutgoingConnection(contactId, + transportId, duplexConnection); + }}); + } }