Moved callback initialisation from start() to constructor so it can be

accessed outside the lock.

api/net/sf/briar/api/transport/TransportPlugin.java

@@ -11,6 +11,11 @@ public interface TransportPlugin {
 	/** Returns the plugin's transport identifier. */
 	TransportId getId();
 
+	/** Starts the plugin. */
+	void start(Map<String, String> localProperties,
+			Map<ContactId, Map<String, String>> remoteProperties,
+			Map<String, String> config) throws IOException;
+
 	/**
 	 * Stops the plugin. No further connections will be passed to the callback
 	 * after this method has returned.

api/net/sf/briar/api/transport/batch/BatchTransportPlugin.java

@@ -1,8 +1,5 @@
 package net.sf.briar.api.transport.batch;
 
-import java.io.IOException;
-import java.util.Map;
-
 import net.sf.briar.api.ContactId;
 import net.sf.briar.api.transport.TransportPlugin;
 
@@ -12,15 +9,6 @@ import net.sf.briar.api.transport.TransportPlugin;
  */
 public interface BatchTransportPlugin extends TransportPlugin {
 
-	/**
-	 * Starts the plugin. Any connections that are later initiated by contacts
-	 * or established through polling will be passed to the given callback.
-	 */
-	void start(Map<String, String> localProperties,
-			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, BatchTransportCallback c)
-	throws IOException;
-
 	/**
 	 * Attempts to create and return a BatchTransportReader for the given
 	 * contact using the current transport and configuration properties.

api/net/sf/briar/api/transport/stream/StreamTransportPlugin.java

@@ -1,8 +1,5 @@
 package net.sf.briar.api.transport.stream;
 
-import java.io.IOException;
-import java.util.Map;
-
 import net.sf.briar.api.ContactId;
 import net.sf.briar.api.transport.TransportPlugin;
 
@@ -12,15 +9,6 @@ import net.sf.briar.api.transport.TransportPlugin;
  */
 public interface StreamTransportPlugin extends TransportPlugin {
 
-	/**
-	 * Starts the plugin. Any connections that are later initiated by contacts
-	 * or established through polling will be passed to the given callback.
-	 */
-	void start(Map<String, String> localProperties,
-			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, StreamTransportCallback c)
-	throws IOException;
-
 	/**
 	 * Attempts to create and return a StreamTransportConnection to the given
 	 * contact using the current transport and configuration properties.

components/net/sf/briar/plugins/AbstractPlugin.java

@@ -26,9 +26,9 @@ public abstract class AbstractPlugin implements TransportPlugin {
 		this.executor = executor;
 	}
 
-	protected synchronized void start(Map<String, String> localProperties,
+	public synchronized void start(Map<String, String> localProperties,
 			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config) {
+			Map<String, String> config) throws IOException {
 		if(started) throw new IllegalStateException();
 		started = true;
 		this.localProperties = Collections.unmodifiableMap(localProperties);

components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java

@@ -36,14 +36,16 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
 	private static final Logger LOG =
 		Logger.getLogger(BluetoothPlugin.class.getName());
 
+	private final StreamTransportCallback callback;
 	private final long pollingInterval;
 
-	private StreamTransportCallback callback = null;
 	private LocalDevice localDevice = null;
 	private StreamConnectionNotifier streamConnectionNotifier = null;
 
-	BluetoothPlugin(Executor executor, long pollingInterval) {
+	BluetoothPlugin(Executor executor, StreamTransportCallback callback,
+			long pollingInterval) {
 		super(executor);
+		this.callback = callback;
 		this.pollingInterval = pollingInterval;
 	}
 
@@ -51,12 +53,11 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
 		return id;
 	}
 
+	@Override
 	public synchronized void start(Map<String, String> localProperties,
 			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, StreamTransportCallback callback)
+			Map<String, String> config) throws IOException {
-	throws IOException {
 		super.start(localProperties, remoteProperties, config);
-		this.callback = callback;
 		// Initialise the Bluetooth stack
 		try {
 			localDevice = LocalDevice.getLocalDevice();
@@ -69,6 +70,7 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
 		executor.execute(createBinder());
 	}
 
+	@Override
 	public synchronized void stop() throws IOException {
 		super.stop();
 		if(streamConnectionNotifier != null) {

components/net/sf/briar/plugins/file/FilePlugin.java

@@ -5,7 +5,6 @@ import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.OutputStream;
-import java.util.Map;
 import java.util.concurrent.Executor;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -26,19 +25,14 @@ implements BatchTransportPlugin {
 	private static final Logger LOG =
 		Logger.getLogger(FilePlugin.class.getName());
 
+	protected final BatchTransportCallback callback;
+
 	protected abstract File chooseOutputDirectory();
 	protected abstract void writerFinished(File f);
 	protected abstract void readerFinished(File f);
 
-	protected FilePlugin(Executor executor) {
+	protected FilePlugin(Executor executor, BatchTransportCallback callback) {
 		super(executor);
-	}
-
-	public synchronized void start(Map<String, String> localProperties,
-			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, BatchTransportCallback callback)
-	throws IOException {
-		super.start(localProperties, remoteProperties, config);
 		this.callback = callback;
 	}
 

components/net/sf/briar/plugins/file/RemovableDrivePlugin.java

@@ -24,9 +24,9 @@ implements RemovableDriveMonitor.Callback {
 	private final RemovableDriveFinder finder;
 	private final RemovableDriveMonitor monitor;
 
-	RemovableDrivePlugin(Executor executor, RemovableDriveFinder finder,
+	RemovableDrivePlugin(Executor executor, BatchTransportCallback callback,
-			RemovableDriveMonitor monitor) {
+			RemovableDriveFinder finder, RemovableDriveMonitor monitor) {
-		super(executor);
+		super(executor, callback);
 		this.finder = finder;
 		this.monitor = monitor;
 	}
@@ -38,9 +38,8 @@ implements RemovableDriveMonitor.Callback {
 	@Override
 	public void start(Map<String, String> localProperties,
 			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, BatchTransportCallback callback)
+			Map<String, String> config) throws IOException {
-	throws IOException {
+		super.start(localProperties, remoteProperties, config);
-		super.start(localProperties, remoteProperties, config, callback);
 		monitor.start(this);
 	}
 

components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java

@@ -11,6 +11,7 @@ import java.util.concurrent.Executor;
 
 import net.sf.briar.api.ContactId;
 import net.sf.briar.api.TransportId;
+import net.sf.briar.api.transport.stream.StreamTransportCallback;
 
 class SimpleSocketPlugin extends SocketPlugin {
 
@@ -20,8 +21,9 @@ class SimpleSocketPlugin extends SocketPlugin {
 
 	private final long pollingInterval;
 
-	SimpleSocketPlugin(Executor executor, long pollingInterval) {
+	SimpleSocketPlugin(Executor executor, StreamTransportCallback callback,
-		super(executor);
+			long pollingInterval) {
+		super(executor, callback);
 		this.pollingInterval = pollingInterval;
 	}
 

components/net/sf/briar/plugins/socket/SocketPlugin.java

@@ -21,8 +21,9 @@ implements StreamTransportPlugin {
 	private static final Logger LOG =
 		Logger.getLogger(SocketPlugin.class.getName());
 
-	// These fields should be accessed with this's lock held
+	protected final StreamTransportCallback callback;
-	protected StreamTransportCallback callback = null;
+
+	// This field should be accessed with this's lock held
 	protected ServerSocket socket = null;
 
 	// These methods should be called with this's lock held and started == true
@@ -32,15 +33,17 @@ implements StreamTransportPlugin {
 	protected abstract Socket createClientSocket() throws IOException;
 	protected abstract ServerSocket createServerSocket() throws IOException;
 
-	protected SocketPlugin(Executor executor) {
+	protected SocketPlugin(Executor executor,
+			StreamTransportCallback callback) {
 		super(executor);
+		this.callback = callback;
 	}
 
+	@Override
 	public synchronized void start(Map<String, String> localProperties,
 			Map<ContactId, Map<String, String>> remoteProperties,
-			Map<String, String> config, StreamTransportCallback callback) {
+			Map<String, String> config) throws IOException {
 		super.start(localProperties, remoteProperties, config);
-		this.callback = callback;
 		executor.execute(createBinder());
 	}
 
@@ -128,6 +131,7 @@ implements StreamTransportPlugin {
 		}
 	}
 
+	@Override
 	public synchronized void stop() throws IOException {
 		super.stop();
 		if(socket != null) {
@@ -136,6 +140,7 @@ implements StreamTransportPlugin {
 		}
 	}
 
+	@Override
 	public synchronized void setLocalProperties(
 			Map<String, String> properties) {
 		super.setLocalProperties(properties);

test/net/sf/briar/plugins/bluetooth/BluetoothClientTest.java

@@ -36,10 +36,10 @@ public class BluetoothClientTest {
 		remoteProperties.put(contactId, properties);
 		// Create the plugin
 		BluetoothPlugin plugin =
-			new BluetoothPlugin(new ImmediateExecutor(), 0L);
+			new BluetoothPlugin(new ImmediateExecutor(), callback, 0L);
 		// Start the plugin
 		System.out.println("Starting plugin");
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 		// Try to connect to the server
 		System.out.println("Creating connection");
 		StreamTransportConnection conn = plugin.createConnection(contactId);

test/net/sf/briar/plugins/bluetooth/BluetoothServerTest.java

@@ -29,10 +29,10 @@ public class BluetoothServerTest {
 		config.put("uuid", UUID);
 		// Create the plugin
 		BluetoothPlugin plugin =
-			new BluetoothPlugin(new ImmediateExecutor(), 0L);
+			new BluetoothPlugin(new ImmediateExecutor(), callback, 0L);
 		// Start the plugin
 		System.out.println("Starting plugin");
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 		// Wait for a connection
 		System.out.println("Waiting for connection");
 		synchronized(callback) {

test/net/sf/briar/plugins/file/RemovableDrivePluginTest.java

@@ -48,13 +48,15 @@ public class RemovableDrivePluginTest extends TestCase {
 	public void testGetId() {
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
 
 		assertEquals(RemovableDrivePlugin.TRANSPORT_ID,
 				plugin.getId().getInt());
@@ -68,12 +70,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -82,8 +84,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		assertNull(plugin.createWriter(contactId));
 
@@ -100,12 +102,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -117,8 +119,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		assertNull(plugin.createWriter(contactId));
 		File[] files = drive1.listFiles();
@@ -137,12 +139,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -154,8 +156,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		assertNull(plugin.createWriter(contactId));
 		File[] files = drive1.listFiles();
@@ -176,12 +178,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -193,8 +195,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		assertNull(plugin.createWriter(contactId));
 		File[] files = drive1.listFiles();
@@ -215,12 +217,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -232,8 +234,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		assertNotNull(plugin.createWriter(contactId));
 		// The output file should exist and should be empty
@@ -257,12 +259,12 @@ public class RemovableDrivePluginTest extends TestCase {
 
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -275,8 +277,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		BatchTransportWriter writer = plugin.createWriter(contactId);
 		assertNotNull(writer);
@@ -303,20 +305,20 @@ public class RemovableDrivePluginTest extends TestCase {
 	public void testEmptyDriveIsIgnored() throws Exception {
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		plugin.driveInserted(testDir);
 
@@ -327,13 +329,15 @@ public class RemovableDrivePluginTest extends TestCase {
 	public void testFilenames() {
 		Mockery context = new Mockery();
 		final Executor executor = context.mock(Executor.class);
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(executor,
-				finder, monitor);
+				callback, finder, monitor);
 
 		assertFalse(plugin.isPossibleConnectionFilename("abcdefg.dat"));
 		assertFalse(plugin.isPossibleConnectionFilename("abcdefghi.dat"));
@@ -348,20 +352,20 @@ public class RemovableDrivePluginTest extends TestCase {
 	@Test
 	public void testSmallFileIsIgnored() throws Exception {
 		Mockery context = new Mockery();
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(
-				new ImmediateExecutor(), finder, monitor);
+				new ImmediateExecutor(), callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		File f = new File(testDir, "abcdefgh.dat");
 		OutputStream out = new FileOutputStream(f);
@@ -377,12 +381,12 @@ public class RemovableDrivePluginTest extends TestCase {
 	@Test
 	public void testReaderIsCreated() throws Exception {
 		Mockery context = new Mockery();
+		final BatchTransportCallback callback =
+			context.mock(BatchTransportCallback.class);
 		final RemovableDriveFinder finder =
 			context.mock(RemovableDriveFinder.class);
 		final RemovableDriveMonitor monitor =
 			context.mock(RemovableDriveMonitor.class);
-		final BatchTransportCallback callback =
-			context.mock(BatchTransportCallback.class);
 
 		context.checking(new Expectations() {{
 			oneOf(monitor).start(with(any(Callback.class)));
@@ -390,8 +394,8 @@ public class RemovableDrivePluginTest extends TestCase {
 		}});
 
 		RemovableDrivePlugin plugin = new RemovableDrivePlugin(
-				new ImmediateExecutor(), finder, monitor);
+				new ImmediateExecutor(), callback, finder, monitor);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 
 		File f = new File(testDir, "abcdefgh.dat");
 		OutputStream out = new FileOutputStream(f);

test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java

@@ -42,8 +42,8 @@ public class SimpleSocketPluginTest extends TestCase {
 		localProperties.put("host", "127.0.0.1");
 		localProperties.put("port", "0");
 		SimpleSocketPlugin plugin =
-			new SimpleSocketPlugin(new ImmediateExecutor(), 10);
+			new SimpleSocketPlugin(new ImmediateExecutor(), callback, 0L);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 		// The plugin should have bound a socket and stored the port number
 		assertNotNull(callback.localProperties);
 		String host = callback.localProperties.get("host");
@@ -76,8 +76,8 @@ public class SimpleSocketPluginTest extends TestCase {
 	public void testOutgoingConnection() throws Exception {
 		StubStreamCallback callback = new StubStreamCallback();
 		SimpleSocketPlugin plugin =
-			new SimpleSocketPlugin(new ImmediateExecutor(), 10);
+			new SimpleSocketPlugin(new ImmediateExecutor(), callback, 0L);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 		// Listen on a local port
 		final ServerSocket ss = new ServerSocket();
 		ss.bind(new InetSocketAddress("127.0.0.1", 0), 10);
@@ -118,8 +118,8 @@ public class SimpleSocketPluginTest extends TestCase {
 		localProperties.put("host", "127.0.0.1");
 		localProperties.put("port", "0");
 		SimpleSocketPlugin plugin =
-			new SimpleSocketPlugin(new ImmediateExecutor(), 10);
+			new SimpleSocketPlugin(new ImmediateExecutor(), callback, 0L);
-		plugin.start(localProperties, remoteProperties, config, callback);
+		plugin.start(localProperties, remoteProperties, config);
 		// The plugin should have bound a socket and stored the port number
 		assertNotNull(callback.localProperties);
 		String host = callback.localProperties.get("host");