From 125ae1b6401e03b635807b8f5e23d0cffd5bb74c Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Sat, 8 Oct 2011 13:13:28 +0100
Subject: [PATCH] Always call the callback outside the plugin's lock.
---
.../briar/api/transport/TransportPlugin.java | 5 +-
.../plugins/bluetooth/BluetoothPlugin.java | 53 +++++++++----------
.../net/sf/briar/plugins/file/FilePlugin.java | 9 +---
.../plugins/socket/SimpleSocketPlugin.java | 7 ++-
.../sf/briar/plugins/socket/SocketPlugin.java | 28 +++-------
5 files changed, 38 insertions(+), 64 deletions(-)
diff --git a/api/net/sf/briar/api/transport/TransportPlugin.java b/api/net/sf/briar/api/transport/TransportPlugin.java
index 89be8c60e3..3f3985fe34 100644
--- a/api/net/sf/briar/api/transport/TransportPlugin.java
+++ b/api/net/sf/briar/api/transport/TransportPlugin.java
@@ -16,10 +16,7 @@ public interface TransportPlugin {
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.
- */
+ /** Stops the plugin. */
void stop() throws IOException;
/** Updates the plugin's local transport properties. */
diff --git a/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java b/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java
index d05977fa8e..2088ed99f0 100644
--- a/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java
+++ b/components/net/sf/briar/plugins/bluetooth/BluetoothPlugin.java
@@ -54,13 +54,15 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
}
@Override
- public synchronized void start(Map<String, String> localProperties,
+ public void start(Map<String, String> localProperties,
Map<ContactId, Map<String, String>> remoteProperties,
Map<String, String> config) throws IOException {
- super.start(localProperties, remoteProperties, config);
// Initialise the Bluetooth stack
try {
- localDevice = LocalDevice.getLocalDevice();
+ synchronized(this) {
+ super.start(localProperties, remoteProperties, config);
+ localDevice = LocalDevice.getLocalDevice();
+ }
} catch(UnsatisfiedLinkError e) {
// On Linux the user may need to install libbluetooth-dev
if(OsUtils.isLinux())
@@ -89,14 +91,16 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
private void bind() {
String uuid;
+ LocalDevice ld;
synchronized(this) {
if(!started) return;
uuid = config.get("uuid");
- if(uuid == null) uuid = createAndSetUuid();
+ ld = localDevice;
}
+ if(uuid == null) uuid = createAndSetUuid();
// Try to make the device discoverable (requires root on Linux)
try {
- localDevice.setDiscoverable(DiscoveryAgent.GIAC);
+ ld.setDiscoverable(DiscoveryAgent.GIAC);
} catch(BluetoothStateException e) {
if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());
}
@@ -120,17 +124,20 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
return;
}
streamConnectionNotifier = scn;
- setLocalBluetoothAddress(localDevice.getBluetoothAddress());
startListener();
}
+ setLocalBluetoothAddress(ld.getBluetoothAddress());
}
private String createAndSetUuid() {
- assert started;
byte[] b = new byte[16];
new Random().nextBytes(b); // FIXME: Use a SecureRandom?
String uuid = StringUtils.toHexString(b);
- Map<String, String> m = new TreeMap<String, String>(config);
+ Map<String, String> m;
+ synchronized(this) {
+ if(!started) return uuid;
+ m = new TreeMap<String, String>(config);
+ }
m.put("uuid", uuid);
callback.setConfig(m);
return uuid;
@@ -160,26 +167,18 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());
return;
}
- synchronized(this) {
- if(!started) {
- try {
- s.close();
- } catch(IOException e) {
- if(LOG.isLoggable(Level.WARNING))
- LOG.warning(e.getMessage());
- }
- return;
- }
- BluetoothTransportConnection conn =
- new BluetoothTransportConnection(s);
- callback.incomingConnectionCreated(conn);
- }
+ BluetoothTransportConnection conn =
+ new BluetoothTransportConnection(s);
+ callback.incomingConnectionCreated(conn);
}
}
private void setLocalBluetoothAddress(String address) {
- assert started;
- Map<String, String> m = new TreeMap<String, String>(localProperties);
+ Map<String, String> m;
+ synchronized(this) {
+ if(!started) return;
+ m = new TreeMap<String, String>(localProperties);
+ }
m.put("address", address);
callback.setLocalProperties(m);
}
@@ -211,11 +210,7 @@ class BluetoothPlugin extends AbstractPlugin implements StreamTransportPlugin {
ContactId c = e.getKey();
String url = e.getValue();
StreamTransportConnection conn = createConnection(c, url);
- if(conn != null) {
- synchronized(this) {
- if(started) callback.outgoingConnectionCreated(c, conn);
- }
- }
+ if(conn != null) callback.outgoingConnectionCreated(c, conn);
}
}
diff --git a/components/net/sf/briar/plugins/file/FilePlugin.java b/components/net/sf/briar/plugins/file/FilePlugin.java
index 4e5698b7ed..3e35a60658 100644
--- a/components/net/sf/briar/plugins/file/FilePlugin.java
+++ b/components/net/sf/briar/plugins/file/FilePlugin.java
@@ -93,15 +93,10 @@ implements BatchTransportPlugin {
if(f.length() < TransportConstants.MIN_CONNECTION_LENGTH) return;
try {
FileInputStream in = new FileInputStream(f);
- synchronized(FilePlugin.this) {
- if(started) {
- callback.readerCreated(new FileTransportReader(f, in,
- FilePlugin.this));
- }
- }
+ callback.readerCreated(new FileTransportReader(f, in,
+ FilePlugin.this));
} catch(IOException e) {
if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());
- return;
}
}
}
diff --git a/components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java b/components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java
index bc0a97a43d..ac0c3691ac 100644
--- a/components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java
+++ b/components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java
@@ -69,14 +69,17 @@ class SimpleSocketPlugin extends SocketPlugin {
@Override
protected void setLocalSocketAddress(SocketAddress s) {
- assert started;
+ Map<String, String> m;
+ synchronized(this) {
+ if(!started) return;
+ m = new TreeMap<String, String>(localProperties);
+ }
if(!(s instanceof InetSocketAddress))
throw new IllegalArgumentException();
InetSocketAddress i = (InetSocketAddress) s;
String host = i.getAddress().getHostAddress();
String port = String.valueOf(i.getPort());
// FIXME: Special handling for private IP addresses?
- Map<String, String> m = new TreeMap<String, String>(localProperties);
m.put("host", host);
m.put("port", port);
callback.setLocalProperties(m);
diff --git a/components/net/sf/briar/plugins/socket/SocketPlugin.java b/components/net/sf/briar/plugins/socket/SocketPlugin.java
index 9be12eb021..bbb6524248 100644
--- a/components/net/sf/briar/plugins/socket/SocketPlugin.java
+++ b/components/net/sf/briar/plugins/socket/SocketPlugin.java
@@ -26,10 +26,11 @@ implements StreamTransportPlugin {
// This field should be accessed with this's lock held
protected ServerSocket socket = null;
+ protected abstract void setLocalSocketAddress(SocketAddress s);
+
// These methods should be called with this's lock held and started == true
protected abstract SocketAddress getLocalSocketAddress();
protected abstract SocketAddress getSocketAddress(ContactId c);
- protected abstract void setLocalSocketAddress(SocketAddress s);
protected abstract Socket createClientSocket() throws IOException;
protected abstract ServerSocket createServerSocket() throws IOException;
@@ -104,8 +105,7 @@ implements StreamTransportPlugin {
ServerSocket ss;
Socket s;
synchronized(this) {
- if(!started) return;
- if(socket == null) return;
+ if(!started || socket == null) return;
ss = socket;
}
try {
@@ -114,20 +114,8 @@ implements StreamTransportPlugin {
if(LOG.isLoggable(Level.WARNING)) LOG.warning(e.getMessage());
return;
}
- synchronized(this) {
- if(!started) {
- try {
- s.close();
- } catch(IOException e) {
- if(LOG.isLoggable(Level.WARNING))
- LOG.warning(e.getMessage());
- }
- return;
- }
- SocketTransportConnection conn =
- new SocketTransportConnection(s);
- callback.incomingConnectionCreated(conn);
- }
+ SocketTransportConnection conn = new SocketTransportConnection(s);
+ callback.incomingConnectionCreated(conn);
}
}
@@ -179,11 +167,7 @@ implements StreamTransportPlugin {
private void connectAndCallBack(ContactId c) {
StreamTransportConnection conn = createConnection(c);
- if(conn != null) {
- synchronized(this) {
- if(started) callback.outgoingConnectionCreated(c, conn);
- }
- }
+ if(conn != null) callback.outgoingConnectionCreated(c, conn);
}
public StreamTransportConnection createConnection(ContactId c) {
--
GitLab