From cd3bacc8cf247dcefe3daa7cf22b932adc8a4b2c Mon Sep 17 00:00:00 2001
From: akwizgran <michael@briarproject.org>
Date: Mon, 10 Dec 2012 16:15:50 +0000
Subject: [PATCH] Code cleanup for modem plugin, better locking in ModemImpl.
---
.../src/net/sf/briar/plugins/modem/Crc32.java | 5 +-
.../src/net/sf/briar/plugins/modem/Frame.java | 6 +-
.../src/net/sf/briar/plugins/modem/Modem.java | 4 +-
.../net/sf/briar/plugins/modem/ModemImpl.java | 85 +++++++++++--------
.../sf/briar/plugins/modem/ModemPlugin.java | 4 +-
5 files changed, 60 insertions(+), 44 deletions(-)
diff --git a/briar-core/src/net/sf/briar/plugins/modem/Crc32.java b/briar-core/src/net/sf/briar/plugins/modem/Crc32.java
index 9cd9d2d81c..8279d3c971 100644
--- a/briar-core/src/net/sf/briar/plugins/modem/Crc32.java
+++ b/briar-core/src/net/sf/briar/plugins/modem/Crc32.java
@@ -15,10 +15,9 @@ class Crc32 {
}
}
- static long update(long c, byte[] b, int off, int len) {
- for(int i = off; i < off + len; i++) {
+ private static long update(long c, byte[] b, int off, int len) {
+ for(int i = off; i < off + len; i++)
c = TABLE[(int) ((c ^ b[i]) & 0xff)] ^ (c >> 8);
- }
return c;
}
diff --git a/briar-core/src/net/sf/briar/plugins/modem/Frame.java b/briar-core/src/net/sf/briar/plugins/modem/Frame.java
index 3feb0e47e6..fc80bd6b88 100644
--- a/briar-core/src/net/sf/briar/plugins/modem/Frame.java
+++ b/briar-core/src/net/sf/briar/plugins/modem/Frame.java
@@ -8,7 +8,7 @@ abstract class Frame {
protected final byte[] buf;
- Frame(byte[] buf) {
+ protected Frame(byte[] buf) {
this.buf = buf;
}
@@ -49,8 +49,8 @@ abstract class Frame {
public boolean equals(Object o) {
if(o instanceof Frame) {
Frame f = (Frame) o;
- if(buf[0] != f.buf[0]) return false;
- return getSequenceNumber() == f.getSequenceNumber();
+ return buf[0] == f.buf[0] &&
+ getSequenceNumber() == f.getSequenceNumber();
}
return false;
}
diff --git a/briar-core/src/net/sf/briar/plugins/modem/Modem.java b/briar-core/src/net/sf/briar/plugins/modem/Modem.java
index ffb372f983..004f7f18be 100644
--- a/briar-core/src/net/sf/briar/plugins/modem/Modem.java
+++ b/briar-core/src/net/sf/briar/plugins/modem/Modem.java
@@ -28,10 +28,10 @@ interface Modem {
boolean dial(String number) throws IOException;
/** Returns a stream for reading from the currently connected call. */
- InputStream getInputStream();
+ InputStream getInputStream() throws IOException;
/** Returns a stream for writing to the currently connected call. */
- OutputStream getOutputStream();
+ OutputStream getOutputStream() throws IOException;
/** Hangs up the modem, ending the currently connected call. */
void hangUp() throws IOException;
diff --git a/briar-core/src/net/sf/briar/plugins/modem/ModemImpl.java b/briar-core/src/net/sf/briar/plugins/modem/ModemImpl.java
index 0fc79fdd2a..ea26b698a3 100644
--- a/briar-core/src/net/sf/briar/plugins/modem/ModemImpl.java
+++ b/briar-core/src/net/sf/briar/plugins/modem/ModemImpl.java
@@ -9,7 +9,6 @@ import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.concurrent.Executor;
-import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.logging.Logger;
@@ -32,20 +31,20 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
private final Executor executor;
private final Callback callback;
private final SerialPort port;
- private final AtomicBoolean initialised, connected;
- private final Semaphore offHook;
+ private final AtomicBoolean initialised; // Locking: self
+ private final AtomicBoolean connected; // Locking: self
private final byte[] line;
private int lineLen = 0;
- private volatile ReliabilityLayer reliabilityLayer = null;
+ private ReliabilityLayer reliabilityLayer = null; // Locking: this
+ private boolean offHook = false; // Locking: this;
ModemImpl(Executor executor, Callback callback, String portName) {
this.executor = executor;
this.callback = callback;
port = new SerialPort(portName);
initialised = new AtomicBoolean(false);
- offHook = new Semaphore(1);
connected = new AtomicBoolean(false);
line = new byte[MAX_LINE_LENGTH];
}
@@ -79,19 +78,19 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
try {
synchronized(initialised) {
if(!initialised.get()) initialised.wait(OK_TIMEOUT);
+ if(initialised.get()) return;
}
} catch(InterruptedException e) {
tryToClose(port);
Thread.currentThread().interrupt();
throw new IOException("Interrupted while initialising modem");
}
- if(!initialised.get())
- throw new IOException("Modem did not respond");
+ tryToClose(port);
+ throw new IOException("Modem did not respond");
}
public void stop() throws IOException {
- if(offHook.tryAcquire()) offHook.release();
- else hangUp();
+ hangUp();
try {
port.closePort();
} catch(SerialPortException e) {
@@ -100,13 +99,16 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
}
public boolean dial(String number) throws IOException {
- if(!offHook.tryAcquire()) {
- if(LOG.isLoggable(INFO))
- LOG.info("Not dialling - call in progress");
- return false;
+ synchronized(this) {
+ if(offHook) {
+ if(LOG.isLoggable(INFO))
+ LOG.info("Not dialling - call in progress");
+ return false;
+ }
+ reliabilityLayer = new ReliabilityLayer(this);
+ reliabilityLayer.start();
+ offHook = true;
}
- reliabilityLayer = new ReliabilityLayer(this);
- reliabilityLayer.start();
if(LOG.isLoggable(INFO)) LOG.info("Dialling");
try {
port.writeBytes(("ATDT" + number + "\r\n").getBytes("US-ASCII"));
@@ -117,36 +119,46 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
try {
synchronized(connected) {
if(!connected.get()) connected.wait(CONNECT_TIMEOUT);
+ if(connected.get()) return true;
}
} catch(InterruptedException e) {
tryToClose(port);
Thread.currentThread().interrupt();
throw new IOException("Interrupted while connecting outgoing call");
}
- if(connected.get()) return true;
hangUp();
return false;
}
- public InputStream getInputStream() {
- return reliabilityLayer.getInputStream();
+ public synchronized InputStream getInputStream() throws IOException {
+ if(offHook) return reliabilityLayer.getInputStream();
+ throw new IOException("Not connected");
}
- public OutputStream getOutputStream() {
- return reliabilityLayer.getOutputStream();
+ public synchronized OutputStream getOutputStream() throws IOException {
+ if(offHook) return reliabilityLayer.getOutputStream();
+ throw new IOException("Not connected");
}
public void hangUp() throws IOException {
+ synchronized(this) {
+ if(!offHook) {
+ if(LOG.isLoggable(INFO))
+ LOG.info("Not hanging up - already on the hook");
+ return;
+ }
+ reliabilityLayer.stop();
+ reliabilityLayer = null;
+ offHook = false;
+ }
if(LOG.isLoggable(INFO)) LOG.info("Hanging up");
+ connected.set(false);
try {
port.setDTR(false);
} catch(SerialPortException e) {
tryToClose(port);
throw new IOException(e.toString());
}
- reliabilityLayer.stop();
- connected.set(false);
- offHook.release();
}
public void handleWrite(byte[] b) throws IOException {
@@ -193,8 +205,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
if(LOG.isLoggable(INFO)) LOG.info("Modem status: " + s);
if(s.startsWith("CONNECT")) {
synchronized(connected) {
- if(!connected.getAndSet(true))
- connected.notifyAll();
+ connected.set(true);
+ connected.notifyAll();
}
// There might be data in the buffer as well as text
int off = i + 1;
@@ -211,8 +223,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
}
} else if(s.equals("OK")) {
synchronized(initialised) {
- if(!initialised.getAndSet(true))
- initialised.notifyAll();
+ initialised.set(true);
+ initialised.notifyAll();
}
} else if(s.equals("RING")) {
executor.execute(new Runnable() {
@@ -233,13 +245,16 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
}
private void answer() throws IOException {
- if(!offHook.tryAcquire()) {
- if(LOG.isLoggable(INFO))
- LOG.info("Not answering - call in progress");
- return;
+ synchronized(this) {
+ if(offHook) {
+ if(LOG.isLoggable(INFO))
+ LOG.info("Not answering - call in progress");
+ return;
+ }
+ reliabilityLayer = new ReliabilityLayer(this);
+ reliabilityLayer.start();
+ offHook = true;
}
- reliabilityLayer = new ReliabilityLayer(this);
- reliabilityLayer.start();
if(LOG.isLoggable(INFO)) LOG.info("Answering");
try {
port.writeBytes("ATA\r\n".getBytes("US-ASCII"));
@@ -247,16 +262,18 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener {
tryToClose(port);
throw new IOException(e.toString());
}
+ boolean success = false;
try {
synchronized(connected) {
if(!connected.get()) connected.wait(CONNECT_TIMEOUT);
+ success = connected.get();
}
} catch(InterruptedException e) {
tryToClose(port);
Thread.currentThread().interrupt();
throw new IOException("Interrupted while connecting incoming call");
}
- if(connected.get()) callback.incomingCallConnected();
+ if(success) callback.incomingCallConnected();
else hangUp();
}
diff --git a/briar-core/src/net/sf/briar/plugins/modem/ModemPlugin.java b/briar-core/src/net/sf/briar/plugins/modem/ModemPlugin.java
index eaa8536bdd..e77444d027 100644
--- a/briar-core/src/net/sf/briar/plugins/modem/ModemPlugin.java
+++ b/briar-core/src/net/sf/briar/plugins/modem/ModemPlugin.java
@@ -208,11 +208,11 @@ class ModemPlugin implements DuplexPlugin, Modem.Callback {
private final CountDownLatch finished = new CountDownLatch(1);
- public InputStream getInputStream() {
+ public InputStream getInputStream() throws IOException {
return modem.getInputStream();
}
- public OutputStream getOutputStream() {
+ public OutputStream getOutputStream() throws IOException {
return modem.getOutputStream();
}
--
GitLab