Moved some work off the UI thread and fixed a potential memory leak.

src/net/sf/briar/api/invitation/InvitationManager.java

@@ -12,6 +12,11 @@ public interface InvitationManager {
 	 */
 	InvitationTask getTask(int handle);
 
-	/** Called by tasks to remove themselves when they terminate. */
+	/** Called by tasks to add themselves to the manager when they start. */
+	void putTask(int handle, InvitationTask task);
+
+	/**
+	 * Called by tasks to remove themselves from the managet when they finish.
+	 */
 	void removeTask(int handle);
 }

src/net/sf/briar/invitation/ConnectorGroup.java

@@ -4,32 +4,41 @@ import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.logging.Level.WARNING;
 import static net.sf.briar.api.plugins.InvitationConstants.CONFIRMATION_TIMEOUT;
 
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.logging.Logger;
 
+import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.invitation.InvitationListener;
 import net.sf.briar.api.invitation.InvitationManager;
 import net.sf.briar.api.invitation.InvitationState;
 import net.sf.briar.api.invitation.InvitationTask;
+import net.sf.briar.api.plugins.PluginManager;
+import net.sf.briar.api.plugins.duplex.DuplexPlugin;
+import net.sf.briar.api.serial.ReaderFactory;
+import net.sf.briar.api.serial.WriterFactory;
 
 /** A task consisting of one or more parallel connection attempts. */
-class ConnectorGroup implements InvitationTask {
+class ConnectorGroup extends Thread implements InvitationTask {
 
 	private static final Logger LOG =
 			Logger.getLogger(ConnectorGroup.class.getName());
 
-	private final InvitationManager manager;
+	private final InvitationManager invitationManager;
+	private final CryptoComponent crypto;
+	private final ReaderFactory readerFactory;
+	private final WriterFactory writerFactory;
+	private final PluginManager pluginManager;
 	private final int handle, localInvitationCode, remoteInvitationCode;
-	private final Collection<Connector> connectors;
 	private final Collection<InvitationListener> listeners;
 	private final AtomicBoolean connected;
 	private final CountDownLatch localConfirmationLatch;
 
 	/*
-	 * All of the following are locking: this. We don't want to call the
+	 * All of the following require locking: this. We don't want to call the
 	 * listeners with a lock held, but we need to avoid a race condition in
 	 * addListener(), so the state that's accessed there after calling
 	 * listeners.add() must be guarded by a lock.
@@ -39,13 +48,18 @@ class ConnectorGroup implements InvitationTask {
 	private boolean localCompared = false, remoteCompared = false;
 	private boolean localMatched = false, remoteMatched = false;
 
-	ConnectorGroup(InvitationManager manager, int handle,
-			int localInvitationCode, int remoteInvitationCode) {
-		this.manager = manager;
+	ConnectorGroup(InvitationManager invitationManager, CryptoComponent crypto,
+			ReaderFactory readerFactory, WriterFactory writerFactory,
+			PluginManager pluginManager, int handle, int localInvitationCode,
+			int remoteInvitationCode) {
+		this.invitationManager = invitationManager;
+		this.crypto = crypto;
+		this.readerFactory = readerFactory;
+		this.writerFactory = writerFactory;
+		this.pluginManager = pluginManager;
 		this.handle = handle;
 		this.localInvitationCode = localInvitationCode;
 		this.remoteInvitationCode = remoteInvitationCode;
-		connectors = new CopyOnWriteArrayList<Connector>();
 		listeners = new CopyOnWriteArrayList<InvitationListener>();
 		connected = new AtomicBoolean(false);
 		localConfirmationLatch = new CountDownLatch(1);
@@ -66,27 +80,50 @@ class ConnectorGroup implements InvitationTask {
 		listeners.remove(l);
 	}
 
-	// FIXME: The task isn't removed from the manager unless this is called
 	public void connect() {
-		for(Connector c : connectors) c.start();
-		new Thread() {
-			@Override
-			public void run() {
-				try {
-					for(Connector c : connectors) c.join();
-				} catch(InterruptedException e) {
-					if(LOG.isLoggable(WARNING))
-						LOG.warning("Interrupted while waiting for connectors");
-				}
-				if(!connected.get()) {
-					synchronized(ConnectorGroup.this) {
-						connectionFailed = true;
-					}
-					for(InvitationListener l : listeners) l.connectionFailed();
-				}
-				manager.removeTask(handle);
+		start();
+	}
+
+	@Override
+	public void run() {
+		// Add this task to the manager
+		invitationManager.putTask(handle, this);
+		// Start the connection threads
+		final Collection<Connector> connectors = new ArrayList<Connector>();
+		// Alice is the party with the smaller invitation code
+		if(localInvitationCode < remoteInvitationCode) {
+			for(DuplexPlugin plugin : pluginManager.getInvitationPlugins()) {
+				Connector c = new AliceConnector(crypto, readerFactory,
+						writerFactory, this, plugin, localInvitationCode,
+						remoteInvitationCode);
+				connectors.add(c);
+				c.start();
 			}
-		}.start();
+		} else {
+			for(DuplexPlugin plugin: pluginManager.getInvitationPlugins()) {
+				Connector c = (new BobConnector(crypto, readerFactory,
+						writerFactory, this, plugin, localInvitationCode,
+						remoteInvitationCode));
+				connectors.add(c);
+				c.start();
+			}
+		}
+		// Wait for the connection threads to finish
+		try {
+			for(Connector c : connectors) c.join();
+		} catch(InterruptedException e) {
+			if(LOG.isLoggable(WARNING))
+				LOG.warning("Interrupted while waiting for connectors");
+		}
+		// If none of the threads connected, inform the listeners
+		if(!connected.get()) {
+			synchronized(this) {
+				connectionFailed = true;
+			}
+			for(InvitationListener l : listeners) l.connectionFailed();
+		}
+		// Remove this task from the manager
+		invitationManager.removeTask(handle);
 	}
 
 	public void localConfirmationSucceeded() {
@@ -104,10 +141,6 @@ class ConnectorGroup implements InvitationTask {
 		localConfirmationLatch.countDown();
 	}
 
-	void addConnector(Connector c) {
-		connectors.add(c);
-	}
-
 	boolean getAndSetConnected() {
 		return connected.getAndSet(true);
 	}

src/net/sf/briar/invitation/InvitationManagerImpl.java

@@ -1,6 +1,5 @@
 package net.sf.briar.invitation;
 
-import java.util.Collection;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
@@ -9,7 +8,6 @@ import net.sf.briar.api.crypto.CryptoComponent;
 import net.sf.briar.api.invitation.InvitationManager;
 import net.sf.briar.api.invitation.InvitationTask;
 import net.sf.briar.api.plugins.PluginManager;
-import net.sf.briar.api.plugins.duplex.DuplexPlugin;
 import net.sf.briar.api.serial.ReaderFactory;
 import net.sf.briar.api.serial.WriterFactory;
 
@@ -37,30 +35,19 @@ class InvitationManagerImpl implements InvitationManager {
 	}
 
 	public InvitationTask createTask(int localCode, int remoteCode) {
-		Collection<DuplexPlugin> plugins = pluginManager.getInvitationPlugins();
 		int handle = nextHandle.incrementAndGet();
-		ConnectorGroup group =
-				new ConnectorGroup(this, handle, localCode, remoteCode);
-		// Alice is the peer with the lesser invitation code
-		if(localCode < remoteCode) {
-			for(DuplexPlugin plugin : plugins) {
-				group.addConnector(new AliceConnector(crypto, readerFactory,
-						writerFactory, group, plugin, localCode, remoteCode));
-			}
-		} else {
-			for(DuplexPlugin plugin : plugins) {
-				group.addConnector(new BobConnector(crypto, readerFactory,
-						writerFactory, group, plugin, localCode, remoteCode));
-			}
-		}
-		tasks.put(handle, group);
-		return group;
+		return new ConnectorGroup(this, crypto, readerFactory, writerFactory,
+				pluginManager, handle, localCode, remoteCode);
 	}
 
 	public InvitationTask getTask(int handle) {
 		return tasks.get(handle);
 	}
 
+	public void putTask(int handle, InvitationTask task) {
+		tasks.put(handle, task);
+	}
+
 	public void removeTask(int handle) {
 		tasks.remove(handle);
 	}