diff --git a/.gitignore b/.gitignore index 91ea7410d8977ededc4a968849a65b194e570976..8fd308ba7977c626cfeb66a6a4f0f6495b7b4934 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,4 @@ build .gradle +.metadata +*.tmp diff --git a/README.md b/README.md new file mode 100644 index 0000000000000000000000000000000000000000..42488858db883ab413418753fa73f7172a6a2b05 --- /dev/null +++ b/README.md @@ -0,0 +1,6 @@ +Briar +======== + +A Fork of https://code.briarproject.org/akwizgran/briar + +[](https://travis-ci.org/AbrahamKiggundu/briar) \ No newline at end of file diff --git a/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java b/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java index 597988cf90ce5f0a31958209de5a1a68866f640e..0e68dff94e8c0883bf1c7f1efcce64fb4028bd83 100644 --- a/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java +++ b/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java @@ -11,6 +11,8 @@ import static java.util.logging.Level.WARNING; import java.util.HashMap; import java.util.Map; import java.util.concurrent.Executor; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import javax.inject.Inject; @@ -56,14 +58,16 @@ Service, EventListener { private final EventBus eventBus; private final Context appContext; private final Map<ContactId, Integer> contactCounts = - new HashMap<ContactId, Integer>(); // Locking: this + new HashMap<ContactId, Integer>(); private final Map<GroupId, Integer> groupCounts = - new HashMap<GroupId, Integer>(); // Locking: this + new HashMap<GroupId, Integer>(); - private int privateTotal = 0, groupTotal = 0; // Locking: this - private int nextRequestId = 0; // Locking: this + private int privateTotal = 0, groupTotal = 0; + private int nextRequestId = 0; private volatile Settings settings = new Settings(); + + private final Lock synchLock = new ReentrantLock(); @Inject public AndroidNotificationManagerImpl(DatabaseComponent db, @@ -103,22 +107,33 @@ Service, EventListener { if(e instanceof SettingsUpdatedEvent) loadSettings(); } - public synchronized void showPrivateMessageNotification(ContactId c) { - Integer count = contactCounts.get(c); - if(count == null) contactCounts.put(c, 1); - else contactCounts.put(c, count + 1); - privateTotal++; - updatePrivateMessageNotification(); + public void showPrivateMessageNotification(ContactId c) { + synchLock.lock(); + try{ + Integer count = contactCounts.get(c); + if(count == null) contactCounts.put(c, 1); + else contactCounts.put(c, count + 1); + privateTotal++; + updatePrivateMessageNotification(); + } + finally{ + synchLock.unlock(); + } } - public synchronized void clearPrivateMessageNotification(ContactId c) { - Integer count = contactCounts.remove(c); - if(count == null) return; // Already cleared - privateTotal -= count; - updatePrivateMessageNotification(); + public void clearPrivateMessageNotification(ContactId c) { + synchLock.lock(); + try{ + Integer count = contactCounts.remove(c); + if(count == null) return; // Already cleared + privateTotal -= count; + updatePrivateMessageNotification(); + } + finally{ + synchLock.unlock(); + } } - // Locking: this private void updatePrivateMessageNotification() { if(privateTotal == 0) { clearPrivateMessageNotification(); @@ -162,7 +177,6 @@ Service, EventListener { } } - // Locking: this private void clearPrivateMessageNotification() { Object o = appContext.getSystemService(NOTIFICATION_SERVICE); NotificationManager nm = (NotificationManager) o; @@ -180,22 +194,33 @@ Service, EventListener { return defaults; } - public synchronized void showGroupPostNotification(GroupId g) { - Integer count = groupCounts.get(g); - if(count == null) groupCounts.put(g, 1); - else groupCounts.put(g, count + 1); - groupTotal++; - updateGroupPostNotification(); + public void showGroupPostNotification(GroupId g) { + synchLock.lock(); + try{ + Integer count = groupCounts.get(g); + if(count == null) groupCounts.put(g, 1); + else groupCounts.put(g, count + 1); + groupTotal++; + updateGroupPostNotification(); + } + finally{ + synchLock.unlock(); + } } - public synchronized void clearGroupPostNotification(GroupId g) { + public void clearGroupPostNotification(GroupId g) { + synchLock.lock(); + try{ Integer count = groupCounts.remove(g); if(count == null) return; // Already cleared groupTotal -= count; updateGroupPostNotification(); + } + finally{ + synchLock.unlock(); + } } - // Locking: this private void updateGroupPostNotification() { if(groupTotal == 0) { clearGroupPostNotification(); @@ -238,18 +263,23 @@ Service, EventListener { } } - // Locking: this private void clearGroupPostNotification() { Object o = appContext.getSystemService(NOTIFICATION_SERVICE); NotificationManager nm = (NotificationManager) o; nm.cancel(GROUP_POST_NOTIFICATION_ID); } - public synchronized void clearNotifications() { - contactCounts.clear(); - groupCounts.clear(); - privateTotal = groupTotal = 0; - clearPrivateMessageNotification(); - clearGroupPostNotification(); + public void clearNotifications() { + synchLock.lock(); + try{ + contactCounts.clear(); + groupCounts.clear(); + privateTotal = groupTotal = 0; + clearPrivateMessageNotification(); + clearGroupPostNotification(); + } + finally{ + synchLock.unlock(); + } } } diff --git a/briar-android/src/org/briarproject/android/ReferenceManagerImpl.java b/briar-android/src/org/briarproject/android/ReferenceManagerImpl.java index e7be8f16741a02ca7a4c8eeb02c00288ab220efa..70f6afe4259515af9eb3c5456286dc05cbe24326 100644 --- a/briar-android/src/org/briarproject/android/ReferenceManagerImpl.java +++ b/briar-android/src/org/briarproject/android/ReferenceManagerImpl.java @@ -4,6 +4,8 @@ import static java.util.logging.Level.INFO; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import org.briarproject.api.android.ReferenceManager; @@ -13,49 +15,70 @@ class ReferenceManagerImpl implements ReferenceManager { private static final Logger LOG = Logger.getLogger(ReferenceManagerImpl.class.getName()); - // Locking: this private final Map<Class<?>, Map<Long, Object>> outerMap = new HashMap<Class<?>, Map<Long, Object>>(); - private long nextHandle = 0; // Locking: this + private long nextHandle = 0; - public synchronized <T> T getReference(long handle, Class<T> c) { - Map<Long, Object> innerMap = outerMap.get(c); - if(innerMap == null) { + private final Lock synchLock = new ReentrantLock(); + + public <T> T getReference(long handle, Class<T> c) { + synchLock.lock(); + try{ + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) { + if(LOG.isLoggable(INFO)) + LOG.info("0 handles for " + c.getName()); + return null; + } if(LOG.isLoggable(INFO)) - LOG.info("0 handles for " + c.getName()); - return null; + LOG.info(innerMap.size() + " handles for " + c.getName()); + Object o = innerMap.get(handle); + return c.cast(o); + } + finally{ + synchLock.unlock(); } - if(LOG.isLoggable(INFO)) - LOG.info(innerMap.size() + " handles for " + c.getName()); - Object o = innerMap.get(handle); - return c.cast(o); + } - public synchronized <T> long putReference(T reference, Class<T> c) { - Map<Long, Object> innerMap = outerMap.get(c); - if(innerMap == null) { - innerMap = new HashMap<Long, Object>(); - outerMap.put(c, innerMap); + public <T> long putReference(T reference, Class<T> c) { + synchLock.lock(); + try{ + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) { + innerMap = new HashMap<Long, Object>(); + outerMap.put(c, innerMap); + } + long handle = nextHandle++; + innerMap.put(handle, reference); + if(LOG.isLoggable(INFO)) { + LOG.info(innerMap.size() + " handles for " + c.getName() + + " after put"); + } + return handle; } - long handle = nextHandle++; - innerMap.put(handle, reference); - if(LOG.isLoggable(INFO)) { - LOG.info(innerMap.size() + " handles for " + c.getName() + - " after put"); + finally{ + synchLock.unlock(); } - return handle; } - public synchronized <T> T removeReference(long handle, Class<T> c) { - Map<Long, Object> innerMap = outerMap.get(c); - if(innerMap == null) return null; - Object o = innerMap.remove(handle); - if(innerMap.isEmpty()) outerMap.remove(c); - if(LOG.isLoggable(INFO)) { - LOG.info(innerMap.size() + " handles for " + c.getName() + - " after remove"); + public <T> T removeReference(long handle, Class<T> c) { + synchLock.lock(); + try{ + Map<Long, Object> innerMap = outerMap.get(c); + if(innerMap == null) return null; + Object o = innerMap.remove(handle); + if(innerMap.isEmpty()) outerMap.remove(c); + if(LOG.isLoggable(INFO)) { + LOG.info(innerMap.size() + " handles for " + c.getName() + + " after remove"); + } + return c.cast(o); } - return c.cast(o); + finally{ + synchLock.unlock(); + } + } } diff --git a/briar-core/src/org/briarproject/crypto/FortunaGenerator.java b/briar-core/src/org/briarproject/crypto/FortunaGenerator.java index 48cf66e5fddfda2fdef5560d435cf9ae62246aa9..9e7ee2dd3d8403bcf59633499c66974ed7094736 100644 --- a/briar-core/src/org/briarproject/crypto/FortunaGenerator.java +++ b/briar-core/src/org/briarproject/crypto/FortunaGenerator.java @@ -1,5 +1,8 @@ package org.briarproject.crypto; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.briarproject.api.crypto.MessageDigest; import org.spongycastle.crypto.BlockCipher; import org.spongycastle.crypto.digests.SHA256Digest; @@ -16,68 +19,95 @@ class FortunaGenerator { private static final int KEY_BYTES = 32; private static final int BLOCK_BYTES = 16; - // All of the following are locking: this private final MessageDigest digest = new DoubleDigest(new SHA256Digest()); private final BlockCipher cipher = new AESLightEngine(); private final byte[] key = new byte[KEY_BYTES]; private final byte[] counter = new byte[BLOCK_BYTES]; private final byte[] buffer = new byte[BLOCK_BYTES]; private final byte[] newKey = new byte[KEY_BYTES]; + + private final Lock synchLock = new ReentrantLock(); FortunaGenerator(byte[] seed) { reseed(seed); } - synchronized void reseed(byte[] seed) { - digest.update(key); - digest.update(seed); - digest.digest(key, 0, KEY_BYTES); - incrementCounter(); + void reseed(byte[] seed) { + synchLock.lock(); + try{ + digest.update(key); + digest.update(seed); + digest.digest(key, 0, KEY_BYTES); + incrementCounter(); + } + finally{ + synchLock.unlock(); + } + } // Package access for testing - synchronized void incrementCounter() { - counter[0]++; - for(int i = 0; counter[i] == 0; i++) { - if(i + 1 == BLOCK_BYTES) - throw new RuntimeException("Counter exhausted"); - counter[i + 1]++; + void incrementCounter() { + synchLock.lock(); + try{ + counter[0]++; + for(int i = 0; counter[i] == 0; i++) { + if(i + 1 == BLOCK_BYTES) + throw new RuntimeException("Counter exhausted"); + counter[i + 1]++; + } } - } + finally{ + synchLock.unlock(); + } +} // Package access for testing - synchronized byte[] getCounter() { - return counter; + byte[] getCounter() { + synchLock.lock(); + try{ + return counter; + } + finally{ + synchLock.unlock(); + } + } - synchronized int nextBytes(byte[] dest, int off, int len) { - // Don't write more than the maximum number of bytes in one request - if(len > MAX_BYTES_PER_REQUEST) len = MAX_BYTES_PER_REQUEST; - cipher.init(true, new KeyParameter(key)); - // Generate full blocks directly into the output buffer - int fullBlocks = len / BLOCK_BYTES; - for(int i = 0; i < fullBlocks; i++) { - cipher.processBlock(counter, 0, dest, off + i * BLOCK_BYTES); - incrementCounter(); + int nextBytes(byte[] dest, int off, int len) { + synchLock.lock(); + try{ + // Don't write more than the maximum number of bytes in one request + if(len > MAX_BYTES_PER_REQUEST) len = MAX_BYTES_PER_REQUEST; + cipher.init(true, new KeyParameter(key)); + // Generate full blocks directly into the output buffer + int fullBlocks = len / BLOCK_BYTES; + for(int i = 0; i < fullBlocks; i++) { + cipher.processBlock(counter, 0, dest, off + i * BLOCK_BYTES); + incrementCounter(); + } + // Generate a partial block if needed + int done = fullBlocks * BLOCK_BYTES, remaining = len - done; + assert remaining < BLOCK_BYTES; + if(remaining > 0) { + cipher.processBlock(counter, 0, buffer, 0); + incrementCounter(); + // Copy the partial block to the output buffer and erase our copy + System.arraycopy(buffer, 0, dest, off + done, remaining); + for(int i = 0; i < BLOCK_BYTES; i++) buffer[i] = 0; + } + // Generate a new key + for(int i = 0; i < KEY_BYTES / BLOCK_BYTES; i++) { + cipher.processBlock(counter, 0, newKey, i * BLOCK_BYTES); + incrementCounter(); + } + System.arraycopy(newKey, 0, key, 0, KEY_BYTES); + for(int i = 0; i < KEY_BYTES; i++) newKey[i] = 0; + // Return the number of bytes written + return len; } - // Generate a partial block if needed - int done = fullBlocks * BLOCK_BYTES, remaining = len - done; - assert remaining < BLOCK_BYTES; - if(remaining > 0) { - cipher.processBlock(counter, 0, buffer, 0); - incrementCounter(); - // Copy the partial block to the output buffer and erase our copy - System.arraycopy(buffer, 0, dest, off + done, remaining); - for(int i = 0; i < BLOCK_BYTES; i++) buffer[i] = 0; - } - // Generate a new key - for(int i = 0; i < KEY_BYTES / BLOCK_BYTES; i++) { - cipher.processBlock(counter, 0, newKey, i * BLOCK_BYTES); - incrementCounter(); + finally{ + synchLock.unlock(); } - System.arraycopy(newKey, 0, key, 0, KEY_BYTES); - for(int i = 0; i < KEY_BYTES; i++) newKey[i] = 0; - // Return the number of bytes written - return len; } } diff --git a/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java b/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java index de83754f9615a548ffe5853c159fe8fce0aa9783..9a0aed9934b394a4aa8b98b84a94c2b651fee833 100644 --- a/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java +++ b/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java @@ -1,5 +1,8 @@ package org.briarproject.crypto; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.briarproject.api.crypto.MessageDigest; import org.briarproject.api.crypto.PseudoRandom; import org.briarproject.util.ByteUtils; @@ -10,6 +13,8 @@ class PseudoRandomImpl implements PseudoRandom { private byte[] state; private int offset; + + private final Lock synchLock = new ReentrantLock(); PseudoRandomImpl(MessageDigest messageDigest, int seed1, int seed2) { this.messageDigest = messageDigest; @@ -21,21 +26,27 @@ class PseudoRandomImpl implements PseudoRandom { offset = 0; } - public synchronized byte[] nextBytes(int bytes) { - byte[] b = new byte[bytes]; - int half = state.length / 2; - int off = 0, len = b.length, available = half - offset; - while(available < len) { - System.arraycopy(state, offset, b, off, available); - off += available; - len -= available; - messageDigest.update(state, half, half); - state = messageDigest.digest(); - offset = 0; - available = half; + public byte[] nextBytes(int bytes) { + synchLock.lock(); + try{ + byte[] b = new byte[bytes]; + int half = state.length / 2; + int off = 0, len = b.length, available = half - offset; + while(available < len) { + System.arraycopy(state, offset, b, off, available); + off += available; + len -= available; + messageDigest.update(state, half, half); + state = messageDigest.digest(); + offset = 0; + available = half; + } + System.arraycopy(state, offset, b, off, len); + offset += len; + return b; + } + finally{ + synchLock.unlock(); } - System.arraycopy(state, offset, b, off, len); - offset += len; - return b; } } diff --git a/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java b/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java index d7cdbdf05d784a596fd594c12bd3c164ffc3fdaa..f37685cd17860279f57d6c2bf7af27c35ba203c6 100644 --- a/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java +++ b/briar-core/src/org/briarproject/crypto/SecretKeyImpl.java @@ -1,5 +1,8 @@ package org.briarproject.crypto; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + import org.briarproject.api.crypto.SecretKey; import org.briarproject.util.ByteUtils; @@ -7,24 +10,39 @@ class SecretKeyImpl implements SecretKey { private final byte[] key; - private boolean erased = false; // Locking: this + private boolean erased = false; + + private final Lock synchLock = new ReentrantLock(); SecretKeyImpl(byte[] key) { this.key = key; } - public synchronized byte[] getEncoded() { - if(erased) throw new IllegalStateException(); - return key; + public byte[] getEncoded() { + synchLock.lock(); + try{ + if(erased) throw new IllegalStateException(); + return key; + } + finally{ + synchLock.unlock(); + } + } public SecretKey copy() { return new SecretKeyImpl(key.clone()); } - public synchronized void erase() { - if(erased) throw new IllegalStateException(); - ByteUtils.erase(key); - erased = true; + public void erase() { + synchLock.lock(); + try{ + if(erased) throw new IllegalStateException(); + ByteUtils.erase(key); + erased = true; + } + finally{ + synchLock.unlock(); + } } } diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index fdeab4e543fb2d5806cf0244978e325bbbaccb03..57b596f7efdde420370cd9a60471b19534ad02a9 100644 --- a/briar-core/src/org/briarproject/db/JdbcDatabase.java +++ b/briar-core/src/org/briarproject/db/JdbcDatabase.java @@ -27,6 +27,9 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import org.briarproject.api.Author; @@ -313,15 +316,18 @@ abstract class JdbcDatabase implements Database<Connection> { private final Clock clock; private final LinkedList<Connection> connections = - new LinkedList<Connection>(); // Locking: self + new LinkedList<Connection>(); private final AtomicInteger transactionCount = new AtomicInteger(0); - private int openConnections = 0; // Locking: connections - private boolean closed = false; // Locking: connections + private int openConnections = 0; + private boolean closed = false; protected abstract Connection createConnection() throws SQLException; protected abstract void flushBuffersToDisk(Statement s) throws SQLException; + + private final Lock connectionsLock = new ReentrantLock(); + private final Condition connectionsChanged = connectionsLock.newCondition(); JdbcDatabase(String hashType, String binaryType, String counterType, String secretType, Clock clock) { @@ -431,19 +437,28 @@ abstract class JdbcDatabase implements Database<Connection> { public Connection startTransaction() throws DbException { Connection txn = null; - synchronized(connections) { + connectionsLock.lock(); + try { if(closed) throw new DbClosedException(); txn = connections.poll(); } + finally{ + connectionsLock.unlock(); + } + try { if(txn == null) { // Open a new connection txn = createConnection(); if(txn == null) throw new DbException(); txn.setAutoCommit(false); - synchronized(connections) { + connectionsLock.lock(); + try { openConnections++; } + finally{ + connectionsLock.unlock(); + } } } catch(SQLException e) { throw new DbException(e); @@ -455,9 +470,13 @@ abstract class JdbcDatabase implements Database<Connection> { public void abortTransaction(Connection txn) { try { txn.rollback(); - synchronized(connections) { + connectionsLock.lock(); + try { connections.add(txn); - connections.notifyAll(); + connectionsChanged.signalAll(); + } + finally{ + connectionsLock.unlock(); } } catch(SQLException e) { // Try to close the connection @@ -468,11 +487,14 @@ abstract class JdbcDatabase implements Database<Connection> { if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e1.toString(), e1); } // Whatever happens, allow the database to close - synchronized(connections) { + connectionsLock.lock(); + try { openConnections--; - connections.notifyAll(); + connectionsChanged.signalAll(); } - } + finally{ + connectionsLock.unlock(); + } } } public void commitTransaction(Connection txn) throws DbException { @@ -486,9 +508,13 @@ abstract class JdbcDatabase implements Database<Connection> { tryToClose(s); throw new DbException(e); } - synchronized(connections) { + connectionsLock.lock(); + try{ connections.add(txn); - connections.notifyAll(); + connectionsChanged.signalAll(); + } + finally{ + connectionsLock.unlock(); } } @@ -502,14 +528,15 @@ abstract class JdbcDatabase implements Database<Connection> { protected void closeAllConnections() throws SQLException { boolean interrupted = false; - synchronized(connections) { + connectionsLock.lock(); + try{ closed = true; for(Connection c : connections) c.close(); openConnections -= connections.size(); connections.clear(); while(openConnections > 0) { try { - connections.wait(); + connectionsChanged.await(); } catch(InterruptedException e) { LOG.warning("Interrupted while closing connections"); interrupted = true; @@ -519,6 +546,10 @@ abstract class JdbcDatabase implements Database<Connection> { connections.clear(); } } + finally{ + connectionsLock.unlock(); + } + if(interrupted) Thread.currentThread().interrupt(); } diff --git a/briar-core/src/org/briarproject/invitation/ConnectorGroup.java b/briar-core/src/org/briarproject/invitation/ConnectorGroup.java index e3c3c6897c9eab0a885161adc41a780516488fc0..a2481915f1034678c682b3fa312add1004792e0f 100644 --- a/briar-core/src/org/briarproject/invitation/ConnectorGroup.java +++ b/briar-core/src/org/briarproject/invitation/ConnectorGroup.java @@ -10,6 +10,8 @@ import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import org.briarproject.api.Author; @@ -60,11 +62,10 @@ class ConnectorGroup extends Thread implements InvitationTask { private final Collection<InvitationListener> listeners; private final AtomicBoolean connected; private final CountDownLatch localConfirmationLatch; + + private final Lock synchLock = new ReentrantLock(); - /* - * 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 in addListener() after + /*The state that's accessed in addListener() after * calling listeners.add() must be guarded by a lock. */ private int localConfirmationCode = -1, remoteConfirmationCode = -1; @@ -104,12 +105,18 @@ class ConnectorGroup extends Thread implements InvitationTask { localConfirmationLatch = new CountDownLatch(1); } - public synchronized InvitationState addListener(InvitationListener l) { - listeners.add(l); - return new InvitationState(localInvitationCode, remoteInvitationCode, - localConfirmationCode, remoteConfirmationCode, connected.get(), - connectionFailed, localCompared, remoteCompared, localMatched, - remoteMatched, remoteName); + public InvitationState addListener(InvitationListener l) { + synchLock.lock(); + try{ + listeners.add(l); + return new InvitationState(localInvitationCode, remoteInvitationCode, + localConfirmationCode, remoteConfirmationCode, connected.get(), + connectionFailed, localCompared, remoteCompared, localMatched, + remoteMatched, remoteName); + } + finally{ + synchLock.unlock(); + } } public void removeListener(InvitationListener l) { @@ -130,9 +137,13 @@ class ConnectorGroup extends Thread implements InvitationTask { localProps = db.getLocalProperties(); } catch(DbException e) { if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - synchronized(this) { + synchLock.lock(); + try { connectionFailed = true; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.connectionFailed(); return; } @@ -163,9 +174,13 @@ class ConnectorGroup extends Thread implements InvitationTask { } // If none of the threads connected, inform the listeners if(!connected.get()) { - synchronized(this) { + synchLock.lock(); + try { connectionFailed = true; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.connectionFailed(); } } @@ -193,18 +208,26 @@ class ConnectorGroup extends Thread implements InvitationTask { } public void localConfirmationSucceeded() { - synchronized(this) { + synchLock.lock(); + try { localCompared = true; localMatched = true; } + finally{ + synchLock.unlock(); + } localConfirmationLatch.countDown(); } public void localConfirmationFailed() { - synchronized(this) { + synchLock.lock(); + try { localCompared = true; localMatched = false; } + finally{ + synchLock.unlock(); + } localConfirmationLatch.countDown(); } @@ -216,10 +239,14 @@ class ConnectorGroup extends Thread implements InvitationTask { } void keyAgreementSucceeded(int localCode, int remoteCode) { - synchronized(this) { + synchLock.lock(); + try { localConfirmationCode = localCode; remoteConfirmationCode = remoteCode; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.keyAgreementSucceeded(localCode, remoteCode); } @@ -230,32 +257,48 @@ class ConnectorGroup extends Thread implements InvitationTask { boolean waitForLocalConfirmationResult() throws InterruptedException { localConfirmationLatch.await(CONFIRMATION_TIMEOUT, MILLISECONDS); - synchronized(this) { + synchLock.lock(); + try { return localMatched; } + finally{ + synchLock.unlock(); + } } void remoteConfirmationSucceeded() { - synchronized(this) { + synchLock.lock(); + try { remoteCompared = true; remoteMatched = true; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.remoteConfirmationSucceeded(); } void remoteConfirmationFailed() { - synchronized(this) { + synchLock.lock(); + try { remoteCompared = true; remoteMatched = false; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.remoteConfirmationFailed(); } void pseudonymExchangeSucceeded(Author remoteAuthor) { String name = remoteAuthor.getName(); - synchronized(this) { + synchLock.lock(); + try { remoteName = name; } + finally{ + synchLock.unlock(); + } for(InvitationListener l : listeners) l.pseudonymExchangeSucceeded(name); } diff --git a/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java b/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java index 153115621572140a95e1714e340a024a9b958287..6427a6d3111c1af44b0f30278555649ae197f5c3 100644 --- a/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java +++ b/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java @@ -2,34 +2,52 @@ package org.briarproject.lifecycle; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.briarproject.api.lifecycle.ShutdownManager; class ShutdownManagerImpl implements ShutdownManager { - protected final Map<Integer, Thread> hooks; // Locking: this + protected final Map<Integer, Thread> hooks; - private int nextHandle = 0; // Locking: this + private int nextHandle = 0; + + private final Lock synchLock = new ReentrantLock(); ShutdownManagerImpl() { hooks = new HashMap<Integer, Thread>(); } - public synchronized int addShutdownHook(Runnable r) { - int handle = nextHandle++; - Thread hook = createThread(r); - hooks.put(handle, hook); - Runtime.getRuntime().addShutdownHook(hook); - return handle; + public int addShutdownHook(Runnable r) { + synchLock.lock(); + try{ + int handle = nextHandle++; + Thread hook = createThread(r); + hooks.put(handle, hook); + Runtime.getRuntime().addShutdownHook(hook); + return handle; + } + finally{ + synchLock.unlock(); + } + } protected Thread createThread(Runnable r) { return new Thread(r, "ShutdownManager"); } - public synchronized boolean removeShutdownHook(int handle) { - Thread hook = hooks.remove(handle); - if(hook == null) return false; - else return Runtime.getRuntime().removeShutdownHook(hook); + public boolean removeShutdownHook(int handle) { + synchLock.lock(); + try{ + Thread hook = hooks.remove(handle); + if(hook == null) return false; + else return Runtime.getRuntime().removeShutdownHook(hook); + } + finally{ + synchLock.unlock(); + } + } } diff --git a/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java b/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java index 2f7d0788968d34e25146237f630a35af58326995..ea9f18a86580eb176fc6eba28f82842cc6dc0d23 100644 --- a/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java +++ b/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java @@ -8,6 +8,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import org.briarproject.api.ContactId; @@ -29,6 +31,8 @@ class ConnectionRegistryImpl implements ConnectionRegistry { private final Map<TransportId, Map<ContactId, Integer>> connections; // Locking: this private final Map<ContactId, Integer> contactCounts; + + private final Lock synchLock = new ReentrantLock(); @Inject ConnectionRegistryImpl(EventBus eventBus) { @@ -40,7 +44,8 @@ class ConnectionRegistryImpl implements ConnectionRegistry { public void registerConnection(ContactId c, TransportId t) { LOG.info("Connection registered"); boolean firstConnection = false; - synchronized(this) { + synchLock.lock(); + try { Map<ContactId, Integer> m = connections.get(t); if(m == null) { m = new HashMap<ContactId, Integer>(); @@ -57,6 +62,10 @@ class ConnectionRegistryImpl implements ConnectionRegistry { contactCounts.put(c, count + 1); } } + finally{ + synchLock.unlock(); + } + if(firstConnection) { LOG.info("Contact connected"); eventBus.broadcast(new ContactConnectedEvent(c)); @@ -66,7 +75,8 @@ class ConnectionRegistryImpl implements ConnectionRegistry { public void unregisterConnection(ContactId c, TransportId t) { LOG.info("Connection unregistered"); boolean lastConnection = false; - synchronized(this) { + synchLock.lock(); + try { Map<ContactId, Integer> m = connections.get(t); if(m == null) throw new IllegalArgumentException(); Integer count = m.remove(c); @@ -85,22 +95,40 @@ class ConnectionRegistryImpl implements ConnectionRegistry { contactCounts.put(c, count - 1); } } + finally{ + synchLock.unlock(); + } + if(lastConnection) { LOG.info("Contact disconnected"); eventBus.broadcast(new ContactDisconnectedEvent(c)); } } - public synchronized Collection<ContactId> getConnectedContacts( + public Collection<ContactId> getConnectedContacts( TransportId t) { - Map<ContactId, Integer> m = connections.get(t); - if(m == null) return Collections.emptyList(); - List<ContactId> ids = new ArrayList<ContactId>(m.keySet()); - if(LOG.isLoggable(INFO)) LOG.info(ids.size() + " contacts connected"); - return Collections.unmodifiableList(ids); + synchLock.lock(); + try{ + Map<ContactId, Integer> m = connections.get(t); + if(m == null) return Collections.emptyList(); + List<ContactId> ids = new ArrayList<ContactId>(m.keySet()); + if(LOG.isLoggable(INFO)) LOG.info(ids.size() + " contacts connected"); + return Collections.unmodifiableList(ids); + } + finally{ + synchLock.unlock(); + } + } - public synchronized boolean isConnected(ContactId c) { - return contactCounts.containsKey(c); + public boolean isConnected(ContactId c) { + synchLock.lock(); + try{ + return contactCounts.containsKey(c); + } + finally{ + synchLock.unlock(); + } + } } diff --git a/briar-core/src/org/briarproject/reliability/Receiver.java b/briar-core/src/org/briarproject/reliability/Receiver.java index be52d7f3b36f1f9763498f0c5425ebe8276588c5..3b2254bf48df2b5265542682ea2b603b4e43f8b0 100644 --- a/briar-core/src/org/briarproject/reliability/Receiver.java +++ b/briar-core/src/org/briarproject/reliability/Receiver.java @@ -5,6 +5,10 @@ import java.util.Comparator; import java.util.Iterator; import java.util.SortedSet; import java.util.TreeSet; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.briarproject.api.reliability.ReadHandler; import org.briarproject.api.system.Clock; @@ -16,13 +20,15 @@ class Receiver implements ReadHandler { private final Clock clock; private final Sender sender; - private final SortedSet<Data> dataFrames; // Locking: this + private final SortedSet<Data> dataFrames; - private int windowSize = MAX_WINDOW_SIZE; // Locking: this + private int windowSize = MAX_WINDOW_SIZE; private long finalSequenceNumber = Long.MAX_VALUE; private long nextSequenceNumber = 1; private volatile boolean valid = true; + private Lock synchLock = new ReentrantLock(); + private Condition dataFrameAvailable = synchLock.newCondition(); Receiver(Clock clock, Sender sender) { this.sender = sender; @@ -30,36 +36,46 @@ class Receiver implements ReadHandler { dataFrames = new TreeSet<Data>(new SequenceNumberComparator()); } - synchronized Data read() throws IOException, InterruptedException { - long now = clock.currentTimeMillis(), end = now + READ_TIMEOUT; - while(now < end && valid) { - if(dataFrames.isEmpty()) { - // Wait for a data frame - wait(end - now); - } else { - Data d = dataFrames.first(); - if(d.getSequenceNumber() == nextSequenceNumber) { - dataFrames.remove(d); - // Update the window - windowSize += d.getPayloadLength(); - sender.sendAck(0, windowSize); - nextSequenceNumber++; - return d; + Data read() throws IOException, InterruptedException { + synchLock.lock(); + try{ + long now = clock.currentTimeMillis(), end = now + READ_TIMEOUT; + while(now < end && valid) { + if(dataFrames.isEmpty()) { + // Wait for a data frame + dataFrameAvailable.await(end - now, TimeUnit.MILLISECONDS); } else { - // Wait for the next in-order data frame - wait(end - now); + Data d = dataFrames.first(); + if(d.getSequenceNumber() == nextSequenceNumber) { + dataFrames.remove(d); + // Update the window + windowSize += d.getPayloadLength(); + sender.sendAck(0, windowSize); + nextSequenceNumber++; + return d; + } else { + // Wait for the next in-order data frame + dataFrameAvailable.await(end - now, TimeUnit.MILLISECONDS); + } } + now = clock.currentTimeMillis(); } - now = clock.currentTimeMillis(); + if(valid) throw new IOException("Read timed out"); + throw new IOException("Connection closed"); + } + finally{ + synchLock.unlock(); } - if(valid) throw new IOException("Read timed out"); - throw new IOException("Connection closed"); } void invalidate() { valid = false; - synchronized(this) { - notifyAll(); + synchLock.lock(); + try { + dataFrameAvailable.signalAll(); + } + finally{ + synchLock.unlock(); } } @@ -79,43 +95,49 @@ class Receiver implements ReadHandler { } } - private synchronized void handleData(byte[] b) throws IOException { - if(b.length < Data.MIN_LENGTH || b.length > Data.MAX_LENGTH) { - // Ignore data frame with invalid length - return; - } - Data d = new Data(b); - int payloadLength = d.getPayloadLength(); - if(payloadLength > windowSize) return; // No space in the window - if(d.getChecksum() != d.calculateChecksum()) { - // Ignore data frame with invalid checksum - return; - } - long sequenceNumber = d.getSequenceNumber(); - if(sequenceNumber == 0) { - // Window probe - } else if(sequenceNumber < nextSequenceNumber) { - // Duplicate data frame - } else if(d.isLastFrame()) { - finalSequenceNumber = sequenceNumber; - // Remove any data frames with higher sequence numbers - Iterator<Data> it = dataFrames.iterator(); - while(it.hasNext()) { - Data d1 = it.next(); - if(d1.getSequenceNumber() >= finalSequenceNumber) it.remove(); + private void handleData(byte[] b) throws IOException { + synchLock.lock(); + try{ + if(b.length < Data.MIN_LENGTH || b.length > Data.MAX_LENGTH) { + // Ignore data frame with invalid length + return; } - if(dataFrames.add(d)) { - windowSize -= payloadLength; - notifyAll(); + Data d = new Data(b); + int payloadLength = d.getPayloadLength(); + if(payloadLength > windowSize) return; // No space in the window + if(d.getChecksum() != d.calculateChecksum()) { + // Ignore data frame with invalid checksum + return; } - } else if(sequenceNumber < finalSequenceNumber) { - if(dataFrames.add(d)) { - windowSize -= payloadLength; - notifyAll(); + long sequenceNumber = d.getSequenceNumber(); + if(sequenceNumber == 0) { + // Window probe + } else if(sequenceNumber < nextSequenceNumber) { + // Duplicate data frame + } else if(d.isLastFrame()) { + finalSequenceNumber = sequenceNumber; + // Remove any data frames with higher sequence numbers + Iterator<Data> it = dataFrames.iterator(); + while(it.hasNext()) { + Data d1 = it.next(); + if(d1.getSequenceNumber() >= finalSequenceNumber) it.remove(); + } + if(dataFrames.add(d)) { + windowSize -= payloadLength; + dataFrameAvailable.signalAll(); + } + } else if(sequenceNumber < finalSequenceNumber) { + if(dataFrames.add(d)) { + windowSize -= payloadLength; + dataFrameAvailable.signalAll(); + } } + // Acknowledge the data frame even if it's a duplicate + sender.sendAck(sequenceNumber, windowSize); + } + finally{ + synchLock.unlock(); } - // Acknowledge the data frame even if it's a duplicate - sender.sendAck(sequenceNumber, windowSize); } private static class SequenceNumberComparator implements Comparator<Data> { diff --git a/briar-core/src/org/briarproject/reliability/Sender.java b/briar-core/src/org/briarproject/reliability/Sender.java index a77ae3d05109666d6e92518bcb75743dd54d005c..97615db73b649dd647f489d9ea8c7a0dfba7b2c4 100644 --- a/briar-core/src/org/briarproject/reliability/Sender.java +++ b/briar-core/src/org/briarproject/reliability/Sender.java @@ -5,6 +5,10 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.briarproject.api.reliability.WriteHandler; import org.briarproject.api.system.Clock; @@ -21,9 +25,8 @@ class Sender { private final Clock clock; private final WriteHandler writeHandler; - private final LinkedList<Outstanding> outstanding; // Locking: this + private final LinkedList<Outstanding> outstanding; - // All of the following are locking: this private int outstandingBytes = 0; private int windowSize = Data.MAX_PAYLOAD_LENGTH; private int rtt = INITIAL_RTT, rttVar = INITIAL_RTT_VAR; @@ -31,6 +34,9 @@ class Sender { private long lastWindowUpdateOrProbe = Long.MAX_VALUE; private boolean dataWaiting = false; + private Lock synchLock = new ReentrantLock(); + private Condition sendWindowAvailable = synchLock.newCondition(); + Sender(Clock clock, WriteHandler writeHandler) { this.clock = clock; this.writeHandler = writeHandler; @@ -58,7 +64,8 @@ class Sender { long sequenceNumber = a.getSequenceNumber(); long now = clock.currentTimeMillis(); Outstanding fastRetransmit = null; - synchronized(this) { + synchLock.lock(); + try { // Remove the acked data frame if it's outstanding int foundIndex = -1; Iterator<Outstanding> it = outstanding.iterator(); @@ -96,6 +103,9 @@ class Sender { // If space has become available, notify any waiting writers if(windowSize > oldWindowSize || foundIndex != -1) notifyAll(); } + finally{ + synchLock.unlock(); + } // Fast retransmission if(fastRetransmit != null) writeHandler.handleWrite(fastRetransmit.data.getBuffer()); @@ -105,7 +115,8 @@ class Sender { long now = clock.currentTimeMillis(); List<Outstanding> retransmit = null; boolean sendProbe = false; - synchronized(this) { + synchLock.lock(); + try { if(outstanding.isEmpty()) { if(dataWaiting && now - lastWindowUpdateOrProbe > rto) { sendProbe = true; @@ -135,6 +146,9 @@ class Sender { } } } + finally{ + synchLock.unlock(); + } // Send a window probe if necessary if(sendProbe) { byte[] buf = new byte[Data.MIN_LENGTH]; @@ -151,12 +165,13 @@ class Sender { void write(Data d) throws IOException, InterruptedException { int payloadLength = d.getPayloadLength(); - synchronized(this) { + synchLock.lock(); + try { // Wait for space in the window long now = clock.currentTimeMillis(), end = now + WRITE_TIMEOUT; while(now < end && outstandingBytes + payloadLength >= windowSize) { dataWaiting = true; - wait(end - now); + sendWindowAvailable.await(end - now, TimeUnit.MILLISECONDS); now = clock.currentTimeMillis(); } if(outstandingBytes + payloadLength >= windowSize) @@ -165,11 +180,20 @@ class Sender { outstandingBytes += payloadLength; dataWaiting = false; } + finally{ + synchLock.unlock(); + } writeHandler.handleWrite(d.getBuffer()); } - synchronized void flush() throws IOException, InterruptedException { - while(dataWaiting || !outstanding.isEmpty()) wait(); + void flush() throws IOException, InterruptedException { + synchLock.lock(); + try{ + while(dataWaiting || !outstanding.isEmpty()) sendWindowAvailable.await(); + } + finally{ + synchLock.unlock(); + } } private static class Outstanding { diff --git a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java index 16db0b2167f094e89cf8ecffe3c33a9b81fd36b0..6ab1688506300266c960a55e95ab3b5534e8ea1b 100644 --- a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java +++ b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java @@ -11,6 +11,8 @@ import java.util.Iterator; import java.util.Map; import java.util.Map.Entry; import java.util.TimerTask; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import javax.inject.Inject; @@ -50,12 +52,13 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { private final Clock clock; private final Timer timer; - // All of the following are locking: this private final Map<TransportId, Long> maxLatencies; private final Map<EndpointKey, TemporarySecret> oldSecrets; private final Map<EndpointKey, TemporarySecret> currentSecrets; private final Map<EndpointKey, TemporarySecret> newSecrets; + private final Lock synchLock = new ReentrantLock(); + @Inject KeyManagerImpl(CryptoComponent crypto, DatabaseComponent db, EventBus eventBus, TagRecogniser tagRecogniser, Clock clock, @@ -72,45 +75,50 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { newSecrets = new HashMap<EndpointKey, TemporarySecret>(); } - public synchronized boolean start() { - eventBus.addListener(this); - // Load the temporary secrets and transport latencies from the database - Collection<TemporarySecret> secrets; + public boolean start() { + synchLock.lock(); try { - secrets = db.getSecrets(); - maxLatencies.putAll(db.getTransportLatencies()); - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - return false; - } - // Work out what phase of its lifecycle each secret is in - long now = clock.currentTimeMillis(); - Collection<TemporarySecret> dead = assignSecretsToMaps(now, secrets); - // Replace any dead secrets - Collection<TemporarySecret> created = replaceDeadSecrets(now, dead); - if(!created.isEmpty()) { - // Store any secrets that have been created, removing any dead ones + eventBus.addListener(this); + // Load the temporary secrets and transport latencies from the database + Collection<TemporarySecret> secrets; try { - db.addSecrets(created); + secrets = db.getSecrets(); + maxLatencies.putAll(db.getTransportLatencies()); } catch(DbException e) { if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); return false; } + // Work out what phase of its lifecycle each secret is in + long now = clock.currentTimeMillis(); + Collection<TemporarySecret> dead = assignSecretsToMaps(now, secrets); + // Replace any dead secrets + Collection<TemporarySecret> created = replaceDeadSecrets(now, dead); + if(!created.isEmpty()) { + // Store any secrets that have been created, removing any dead ones + try { + db.addSecrets(created); + } catch(DbException e) { + if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + return false; + } + } + // Pass the old, current and new secrets to the recogniser + for(TemporarySecret s : oldSecrets.values()) + tagRecogniser.addSecret(s); + for(TemporarySecret s : currentSecrets.values()) + tagRecogniser.addSecret(s); + for(TemporarySecret s : newSecrets.values()) + tagRecogniser.addSecret(s); + // Schedule periodic key rotation + timer.scheduleAtFixedRate(this, MS_BETWEEN_CHECKS, MS_BETWEEN_CHECKS); + return true; + } + finally{ + synchLock.unlock(); } - // Pass the old, current and new secrets to the recogniser - for(TemporarySecret s : oldSecrets.values()) - tagRecogniser.addSecret(s); - for(TemporarySecret s : currentSecrets.values()) - tagRecogniser.addSecret(s); - for(TemporarySecret s : newSecrets.values()) - tagRecogniser.addSecret(s); - // Schedule periodic key rotation - timer.scheduleAtFixedRate(this, MS_BETWEEN_CHECKS, MS_BETWEEN_CHECKS); - return true; } // Assigns secrets to the appropriate maps and returns any dead secrets - // Locking: this private Collection<TemporarySecret> assignSecretsToMaps(long now, Collection<TemporarySecret> secrets) { Collection<TemporarySecret> dead = new ArrayList<TemporarySecret>(); @@ -144,7 +152,6 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } // Replaces and erases the given secrets and returns any secrets created - // Locking: this private Collection<TemporarySecret> replaceDeadSecrets(long now, Collection<TemporarySecret> dead) { // If there are several dead secrets for an endpoint, use the newest @@ -215,115 +222,138 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { return created; } - public synchronized boolean stop() { - eventBus.removeListener(this); - timer.cancel(); - tagRecogniser.removeSecrets(); - maxLatencies.clear(); - removeAndEraseSecrets(oldSecrets); - removeAndEraseSecrets(currentSecrets); - removeAndEraseSecrets(newSecrets); - return true; + public boolean stop() { + synchLock.lock(); + try{ + eventBus.removeListener(this); + timer.cancel(); + tagRecogniser.removeSecrets(); + maxLatencies.clear(); + removeAndEraseSecrets(oldSecrets); + removeAndEraseSecrets(currentSecrets); + removeAndEraseSecrets(newSecrets); + return true; + } + finally{ + synchLock.unlock(); + } } - // Locking: this private void removeAndEraseSecrets(Map<?, TemporarySecret> m) { for(TemporarySecret s : m.values()) ByteUtils.erase(s.getSecret()); m.clear(); } - public synchronized StreamContext getStreamContext(ContactId c, + public StreamContext getStreamContext(ContactId c, TransportId t) { - TemporarySecret s = currentSecrets.get(new EndpointKey(c, t)); - if(s == null) { - LOG.info("No secret for endpoint"); - return null; - } - long streamNumber; - try { - streamNumber = db.incrementStreamCounter(c, t, s.getPeriod()); - if(streamNumber == -1) { - LOG.info("No counter for period"); + synchLock.lock(); + try{ + TemporarySecret s = currentSecrets.get(new EndpointKey(c, t)); + if(s == null) { + LOG.info("No secret for endpoint"); + return null; + } + long streamNumber; + try { + streamNumber = db.incrementStreamCounter(c, t, s.getPeriod()); + if(streamNumber == -1) { + LOG.info("No counter for period"); + return null; + } + } catch(DbException e) { + if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); return null; } - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - return null; + // Clone the secret - the original will be erased + byte[] secret = s.getSecret().clone(); + return new StreamContext(c, t, secret, streamNumber, s.getAlice()); + } + finally{ + synchLock.unlock(); } - // Clone the secret - the original will be erased - byte[] secret = s.getSecret().clone(); - return new StreamContext(c, t, secret, streamNumber, s.getAlice()); } - public synchronized void endpointAdded(Endpoint ep, long maxLatency, + public void endpointAdded(Endpoint ep, long maxLatency, byte[] initialSecret) { - maxLatencies.put(ep.getTransportId(), maxLatency); - // Work out which rotation period we're in - long elapsed = clock.currentTimeMillis() - ep.getEpoch(); - long rotation = maxLatency + MAX_CLOCK_DIFFERENCE; - long period = (elapsed / rotation) + 1; - if(period < 1) throw new IllegalStateException(); - // Derive the old, current and new secrets - byte[] b1 = initialSecret; - for(long p = 0; p < period; p++) { - byte[] temp = crypto.deriveNextSecret(b1, p); - ByteUtils.erase(b1); - b1 = temp; + synchLock.lock(); + try{ + maxLatencies.put(ep.getTransportId(), maxLatency); + // Work out which rotation period we're in + long elapsed = clock.currentTimeMillis() - ep.getEpoch(); + long rotation = maxLatency + MAX_CLOCK_DIFFERENCE; + long period = (elapsed / rotation) + 1; + if(period < 1) throw new IllegalStateException(); + // Derive the old, current and new secrets + byte[] b1 = initialSecret; + for(long p = 0; p < period; p++) { + byte[] temp = crypto.deriveNextSecret(b1, p); + ByteUtils.erase(b1); + b1 = temp; + } + byte[] b2 = crypto.deriveNextSecret(b1, period); + byte[] b3 = crypto.deriveNextSecret(b2, period + 1); + TemporarySecret s1 = new TemporarySecret(ep, period - 1, b1); + TemporarySecret s2 = new TemporarySecret(ep, period, b2); + TemporarySecret s3 = new TemporarySecret(ep, period + 1, b3); + // Add the incoming secrets to their respective maps + EndpointKey k = new EndpointKey(ep); + oldSecrets.put(k, s1); + currentSecrets.put(k, s2); + newSecrets.put(k, s3); + // Store the new secrets + try { + db.addSecrets(Arrays.asList(s1, s2, s3)); + } catch(DbException e) { + if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + return; + } + // Pass the new secrets to the recogniser + tagRecogniser.addSecret(s1); + tagRecogniser.addSecret(s2); + tagRecogniser.addSecret(s3); } - byte[] b2 = crypto.deriveNextSecret(b1, period); - byte[] b3 = crypto.deriveNextSecret(b2, period + 1); - TemporarySecret s1 = new TemporarySecret(ep, period - 1, b1); - TemporarySecret s2 = new TemporarySecret(ep, period, b2); - TemporarySecret s3 = new TemporarySecret(ep, period + 1, b3); - // Add the incoming secrets to their respective maps - EndpointKey k = new EndpointKey(ep); - oldSecrets.put(k, s1); - currentSecrets.put(k, s2); - newSecrets.put(k, s3); - // Store the new secrets - try { - db.addSecrets(Arrays.asList(s1, s2, s3)); - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - return; + finally{ + synchLock.unlock(); } - // Pass the new secrets to the recogniser - tagRecogniser.addSecret(s1); - tagRecogniser.addSecret(s2); - tagRecogniser.addSecret(s3); } @Override - public synchronized void run() { + public void run() { + synchLock.lock(); + try{ // Rebuild the maps because we may be running a whole period late - Collection<TemporarySecret> secrets = new ArrayList<TemporarySecret>(); - secrets.addAll(oldSecrets.values()); - secrets.addAll(currentSecrets.values()); - secrets.addAll(newSecrets.values()); - oldSecrets.clear(); - currentSecrets.clear(); - newSecrets.clear(); - // Work out what phase of its lifecycle each secret is in - long now = clock.currentTimeMillis(); - Collection<TemporarySecret> dead = assignSecretsToMaps(now, secrets); - // Remove any dead secrets from the recogniser - for(TemporarySecret s : dead) { - ContactId c = s.getContactId(); - TransportId t = s.getTransportId(); - long period = s.getPeriod(); - tagRecogniser.removeSecret(c, t, period); - } - // Replace any dead secrets - Collection<TemporarySecret> created = replaceDeadSecrets(now, dead); - if(!created.isEmpty()) { - // Store any secrets that have been created - try { - db.addSecrets(created); - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + Collection<TemporarySecret> secrets = new ArrayList<TemporarySecret>(); + secrets.addAll(oldSecrets.values()); + secrets.addAll(currentSecrets.values()); + secrets.addAll(newSecrets.values()); + oldSecrets.clear(); + currentSecrets.clear(); + newSecrets.clear(); + // Work out what phase of its lifecycle each secret is in + long now = clock.currentTimeMillis(); + Collection<TemporarySecret> dead = assignSecretsToMaps(now, secrets); + // Remove any dead secrets from the recogniser + for(TemporarySecret s : dead) { + ContactId c = s.getContactId(); + TransportId t = s.getTransportId(); + long period = s.getPeriod(); + tagRecogniser.removeSecret(c, t, period); } - // Pass any secrets that have been created to the recogniser - for(TemporarySecret s : created) tagRecogniser.addSecret(s); + // Replace any dead secrets + Collection<TemporarySecret> created = replaceDeadSecrets(now, dead); + if(!created.isEmpty()) { + // Store any secrets that have been created + try { + db.addSecrets(created); + } catch(DbException e) { + if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); + } + // Pass any secrets that have been created to the recogniser + for(TemporarySecret s : created) tagRecogniser.addSecret(s); + } + } + finally{ + synchLock.unlock(); } } @@ -340,7 +370,6 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } } - // Locking: this private void removeAndEraseSecrets(ContactId c, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); while(it.hasNext()) { @@ -352,7 +381,6 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } } - // Locking: this private void removeAndEraseSecrets(TransportId t, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); @@ -407,11 +435,15 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { public void run() { ContactId c = event.getContactId(); tagRecogniser.removeSecrets(c); - synchronized(KeyManagerImpl.this) { + synchLock.lock(); + try { removeAndEraseSecrets(c, oldSecrets); removeAndEraseSecrets(c, currentSecrets); removeAndEraseSecrets(c, newSecrets); } + finally{ + synchLock.unlock(); + } } } @@ -425,9 +457,13 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { @Override public void run() { - synchronized(KeyManagerImpl.this) { + synchLock.lock(); + try { maxLatencies.put(event.getTransportId(), event.getMaxLatency()); } + finally{ + synchLock.unlock(); + } } } @@ -443,12 +479,16 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { public void run() { TransportId t = event.getTransportId(); tagRecogniser.removeSecrets(t); - synchronized(KeyManagerImpl.this) { + synchLock.lock(); + try { maxLatencies.remove(t); removeAndEraseSecrets(t, oldSecrets); removeAndEraseSecrets(t, currentSecrets); removeAndEraseSecrets(t, newSecrets); } + finally{ + synchLock.unlock(); + } } } } diff --git a/briar-core/src/org/briarproject/transport/TagRecogniserImpl.java b/briar-core/src/org/briarproject/transport/TagRecogniserImpl.java index a929bc2d985cdadef26696823166f274de093f7a..30d0054acaa6b749c40ae2d4d3b6418cd1ec0aa9 100644 --- a/briar-core/src/org/briarproject/transport/TagRecogniserImpl.java +++ b/briar-core/src/org/briarproject/transport/TagRecogniserImpl.java @@ -2,6 +2,8 @@ package org.briarproject.transport; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import javax.inject.Inject; @@ -18,8 +20,11 @@ class TagRecogniserImpl implements TagRecogniser { private final CryptoComponent crypto; private final DatabaseComponent db; - // Locking: this + private final Map<TransportId, TransportTagRecogniser> recognisers; + + private final Lock synchLock = new ReentrantLock(); + @Inject TagRecogniserImpl(CryptoComponent crypto, DatabaseComponent db) { @@ -31,9 +36,13 @@ class TagRecogniserImpl implements TagRecogniser { public StreamContext recogniseTag(TransportId t, byte[] tag) throws DbException { TransportTagRecogniser r; - synchronized(this) { + synchLock.lock(); + try { r = recognisers.get(t); } + finally{ + synchLock.unlock(); + } if(r == null) return null; return r.recogniseTag(tag); } @@ -41,35 +50,63 @@ class TagRecogniserImpl implements TagRecogniser { public void addSecret(TemporarySecret s) { TransportId t = s.getTransportId(); TransportTagRecogniser r; - synchronized(this) { + synchLock.lock(); + try { r = recognisers.get(t); if(r == null) { r = new TransportTagRecogniser(crypto, db, t); recognisers.put(t, r); } } + finally{ + synchLock.unlock(); + } r.addSecret(s); } public void removeSecret(ContactId c, TransportId t, long period) { TransportTagRecogniser r; - synchronized(this) { + synchLock.lock(); + try { r = recognisers.get(t); } + finally{ + synchLock.unlock(); + } if(r != null) r.removeSecret(c, period); } - public synchronized void removeSecrets(ContactId c) { - for(TransportTagRecogniser r : recognisers.values()) - r.removeSecrets(c); + public void removeSecrets(ContactId c) { + synchLock.lock(); + try{ + for(TransportTagRecogniser r : recognisers.values()) + r.removeSecrets(c); + } + finally{ + synchLock.unlock(); + } } - public synchronized void removeSecrets(TransportId t) { - recognisers.remove(t); + public void removeSecrets(TransportId t) { + synchLock.lock(); + try{ + recognisers.remove(t); + } + finally{ + synchLock.unlock(); + } + } - public synchronized void removeSecrets() { - for(TransportTagRecogniser r : recognisers.values()) - r.removeSecrets(); + public void removeSecrets() { + synchLock.lock(); + try{ + for(TransportTagRecogniser r : recognisers.values()) + r.removeSecrets(); + } + finally{ + synchLock.unlock(); + } + } } diff --git a/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java b/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java index 3c9508397c757219c331a469b0c61489a0c7fde1..353681d498280ac4260fd40ac670439407974a3a 100644 --- a/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java +++ b/briar-core/src/org/briarproject/transport/TransportTagRecogniser.java @@ -6,6 +6,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import org.briarproject.api.Bytes; import org.briarproject.api.ContactId; @@ -27,8 +29,10 @@ class TransportTagRecogniser { private final CryptoComponent crypto; private final DatabaseComponent db; private final TransportId transportId; - private final Map<Bytes, TagContext> tagMap; // Locking: this - private final Map<RemovalKey, RemovalContext> removalMap; // Locking: this + private final Map<Bytes, TagContext> tagMap; + private final Map<RemovalKey, RemovalContext> removalMap; + + private final Lock synchLock = new ReentrantLock(); TransportTagRecogniser(CryptoComponent crypto, DatabaseComponent db, TransportId transportId) { @@ -39,65 +43,82 @@ class TransportTagRecogniser { removalMap = new HashMap<RemovalKey, RemovalContext>(); } - synchronized StreamContext recogniseTag(byte[] tag) throws DbException { - TagContext t = tagMap.remove(new Bytes(tag)); - if(t == null) return null; // The tag was not expected - // Update the reordering window and the expected tags - SecretKey key = crypto.deriveTagKey(t.secret, !t.alice); - for(long streamNumber : t.window.setSeen(t.streamNumber)) { - byte[] tag1 = new byte[TAG_LENGTH]; - crypto.encodeTag(tag1, key, streamNumber); - if(streamNumber < t.streamNumber) { - TagContext removed = tagMap.remove(new Bytes(tag1)); - assert removed != null; - } else { - TagContext added = new TagContext(t, streamNumber); - TagContext duplicate = tagMap.put(new Bytes(tag1), added); - assert duplicate == null; + StreamContext recogniseTag(byte[] tag) throws DbException { + synchLock.lock(); + try{ + TagContext t = tagMap.remove(new Bytes(tag)); + if(t == null) return null; // The tag was not expected + // Update the reordering window and the expected tags + SecretKey key = crypto.deriveTagKey(t.secret, !t.alice); + for(long streamNumber : t.window.setSeen(t.streamNumber)) { + byte[] tag1 = new byte[TAG_LENGTH]; + crypto.encodeTag(tag1, key, streamNumber); + if(streamNumber < t.streamNumber) { + TagContext removed = tagMap.remove(new Bytes(tag1)); + assert removed != null; + } else { + TagContext added = new TagContext(t, streamNumber); + TagContext duplicate = tagMap.put(new Bytes(tag1), added); + assert duplicate == null; + } } + key.erase(); + // Store the updated reordering window in the DB + db.setReorderingWindow(t.contactId, transportId, t.period, + t.window.getCentre(), t.window.getBitmap()); + // Clone the secret - the key manager will erase the original + byte[] secret = t.secret.clone(); + return new StreamContext(t.contactId, transportId, secret, + t.streamNumber, t.alice); + } + finally{ + synchLock.unlock(); } - key.erase(); - // Store the updated reordering window in the DB - db.setReorderingWindow(t.contactId, transportId, t.period, - t.window.getCentre(), t.window.getBitmap()); - // Clone the secret - the key manager will erase the original - byte[] secret = t.secret.clone(); - return new StreamContext(t.contactId, transportId, secret, - t.streamNumber, t.alice); } - synchronized void addSecret(TemporarySecret s) { - ContactId contactId = s.getContactId(); - boolean alice = s.getAlice(); - long period = s.getPeriod(); - byte[] secret = s.getSecret(); - long centre = s.getWindowCentre(); - byte[] bitmap = s.getWindowBitmap(); - // Create the reordering window and the expected tags - SecretKey key = crypto.deriveTagKey(secret, !alice); - ReorderingWindow window = new ReorderingWindow(centre, bitmap); - for(long streamNumber : window.getUnseen()) { - byte[] tag = new byte[TAG_LENGTH]; - crypto.encodeTag(tag, key, streamNumber); - TagContext added = new TagContext(contactId, alice, period, - secret, window, streamNumber); - TagContext duplicate = tagMap.put(new Bytes(tag), added); - assert duplicate == null; + void addSecret(TemporarySecret s) { + synchLock.lock(); + try{ + ContactId contactId = s.getContactId(); + boolean alice = s.getAlice(); + long period = s.getPeriod(); + byte[] secret = s.getSecret(); + long centre = s.getWindowCentre(); + byte[] bitmap = s.getWindowBitmap(); + // Create the reordering window and the expected tags + SecretKey key = crypto.deriveTagKey(secret, !alice); + ReorderingWindow window = new ReorderingWindow(centre, bitmap); + for(long streamNumber : window.getUnseen()) { + byte[] tag = new byte[TAG_LENGTH]; + crypto.encodeTag(tag, key, streamNumber); + TagContext added = new TagContext(contactId, alice, period, + secret, window, streamNumber); + TagContext duplicate = tagMap.put(new Bytes(tag), added); + assert duplicate == null; + } + key.erase(); + // Create a removal context to remove the window and the tags later + RemovalContext r = new RemovalContext(window, secret, alice); + removalMap.put(new RemovalKey(contactId, period), r); + } + finally{ + synchLock.unlock(); } - key.erase(); - // Create a removal context to remove the window and the tags later - RemovalContext r = new RemovalContext(window, secret, alice); - removalMap.put(new RemovalKey(contactId, period), r); } - synchronized void removeSecret(ContactId contactId, long period) { - RemovalKey k = new RemovalKey(contactId, period); - RemovalContext removed = removalMap.remove(k); - if(removed == null) throw new IllegalArgumentException(); - removeSecret(removed); + void removeSecret(ContactId contactId, long period) { + synchLock.lock(); + try{ + RemovalKey k = new RemovalKey(contactId, period); + RemovalContext removed = removalMap.remove(k); + if(removed == null) throw new IllegalArgumentException(); + removeSecret(removed); + } + finally{ + synchLock.unlock(); + } } - // Locking: this private void removeSecret(RemovalContext r) { // Remove the expected tags SecretKey key = crypto.deriveTagKey(r.secret, !r.alice); @@ -110,17 +131,29 @@ class TransportTagRecogniser { key.erase(); } - synchronized void removeSecrets(ContactId c) { - Collection<RemovalKey> keysToRemove = new ArrayList<RemovalKey>(); - for(RemovalKey k : removalMap.keySet()) - if(k.contactId.equals(c)) keysToRemove.add(k); - for(RemovalKey k : keysToRemove) removeSecret(k.contactId, k.period); + void removeSecrets(ContactId c) { + synchLock.lock(); + try{ + Collection<RemovalKey> keysToRemove = new ArrayList<RemovalKey>(); + for(RemovalKey k : removalMap.keySet()) + if(k.contactId.equals(c)) keysToRemove.add(k); + for(RemovalKey k : keysToRemove) removeSecret(k.contactId, k.period); + } + finally{ + synchLock.unlock(); + } } - synchronized void removeSecrets() { - for(RemovalContext r : removalMap.values()) removeSecret(r); - assert tagMap.isEmpty(); - removalMap.clear(); + void removeSecrets() { + synchLock.lock(); + try{ + for(RemovalContext r : removalMap.values()) removeSecret(r); + assert tagMap.isEmpty(); + removalMap.clear(); + } + finally{ + synchLock.unlock(); + } } private static class TagContext { diff --git a/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java b/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java index 5b1715cfa559154fe698d77373fb1bc5455f2f7a..bb21e6c4b37aa7168a068f42539a5ca0fb4085e5 100644 --- a/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java +++ b/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java @@ -8,6 +8,8 @@ import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import org.briarproject.util.OsUtils; @@ -37,7 +39,10 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { private final Map<String, Object> options; - private boolean initialised = false; // Locking: this + private boolean initialised = false; + + private final Lock synchLock = new ReentrantLock(); + WindowsShutdownManagerImpl() { // Use the Unicode versions of Win32 API calls @@ -48,9 +53,15 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { } @Override - public synchronized int addShutdownHook(Runnable r) { - if(!initialised) initialise(); - return super.addShutdownHook(r); + public int addShutdownHook(Runnable r) { + synchLock.lock(); + try { + if(!initialised) initialise(); + return super.addShutdownHook(r); + } + finally{ + synchLock.unlock(); + } } @Override @@ -58,7 +69,6 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { return new StartOnce(r); } - // Locking: this private void initialise() { if(OsUtils.isWindows()) { new EventLoop().start(); @@ -69,20 +79,26 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { } // Package access for testing - synchronized void runShutdownHooks() { - boolean interrupted = false; - // Start each hook in its own thread - for(Thread hook : hooks.values()) hook.start(); - // Wait for all the hooks to finish - for(Thread hook : hooks.values()) { - try { - hook.join(); - } catch(InterruptedException e) { - LOG.warning("Interrupted while running shutdown hooks"); - interrupted = true; + void runShutdownHooks() { + synchLock.lock(); + try { + boolean interrupted = false; + // Start each hook in its own thread + for(Thread hook : hooks.values()) hook.start(); + // Wait for all the hooks to finish + for(Thread hook : hooks.values()) { + try { + hook.join(); + } catch(InterruptedException e) { + LOG.warning("Interrupted while running shutdown hooks"); + interrupted = true; + } } + if(interrupted) Thread.currentThread().interrupt(); + } + finally{ + synchLock.unlock(); } - if(interrupted) Thread.currentThread().interrupt(); } private class EventLoop extends Thread { diff --git a/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java b/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java index 2d8fd287616093ff0ddd54b5c5f9adbdd75ad412..375e22d5af7df4e678b4b107c10c26308ef9a0ae 100644 --- a/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java +++ b/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java @@ -4,6 +4,10 @@ import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.concurrent.Executor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { @@ -14,11 +18,14 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { private final Executor ioExecutor; private final RemovableDriveFinder finder; private final long pollingInterval; - private final Object pollingLock = new Object(); private volatile boolean running = false; private volatile Callback callback = null; + private final Lock synchLock = new ReentrantLock(); + private final Condition stopPolling = synchLock.newCondition(); + + public PollingRemovableDriveMonitor(Executor ioExecutor, RemovableDriveFinder finder, long pollingInterval) { this.ioExecutor = ioExecutor; @@ -34,8 +41,12 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { public void stop() throws IOException { running = false; - synchronized(pollingLock) { - pollingLock.notifyAll(); + synchLock.lock(); + try { + stopPolling.signalAll(); + } + finally { + synchLock.unlock(); } } @@ -43,8 +54,12 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { try { Collection<File> drives = finder.findRemovableDrives(); while(running) { - synchronized(pollingLock) { - pollingLock.wait(pollingInterval); + synchLock.lock(); + try { + stopPolling.await(pollingInterval, TimeUnit.MILLISECONDS); + } + finally{ + synchLock.unlock(); } if(!running) return; Collection<File> newDrives = finder.findRemovableDrives(); diff --git a/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java b/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java index 4d9e95df938c2c0f43b083e95562d10afe11cd11..00335b94746c2e952c5c46c92a16d46cf2839764 100644 --- a/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java +++ b/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java @@ -4,6 +4,9 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import net.contentobjects.jnotify.JNotify; import net.contentobjects.jnotify.JNotifyListener; @@ -11,17 +14,20 @@ import net.contentobjects.jnotify.JNotifyListener; abstract class UnixRemovableDriveMonitor implements RemovableDriveMonitor, JNotifyListener { - private static boolean triedLoad = false; // Locking: class - private static Throwable loadError = null; // Locking: class + private static boolean triedLoad = false; + private static Throwable loadError = null; - // Locking: this private final List<Integer> watches = new ArrayList<Integer>(); - private boolean started = false; // Locking: this - private Callback callback = null; // Locking: this + private boolean started = false; + private Callback callback = null; protected abstract String[] getPathsToWatch(); - + + //TODO: rationalise this in a further refactor + private final Lock synchLock = new ReentrantLock(); + private static final Lock staticSynchLock = new ReentrantLock(); + private static Throwable tryLoad() { try { Class.forName("net.contentobjects.jnotify.JNotify"); @@ -33,12 +39,18 @@ JNotifyListener { } } - public static synchronized void checkEnabled() throws IOException { - if(!triedLoad) { - loadError = tryLoad(); - triedLoad = true; + public static void checkEnabled() throws IOException { + staticSynchLock.lock(); + try { + if(!triedLoad) { + loadError = tryLoad(); + triedLoad = true; + } + if(loadError != null) throw new IOException(loadError.toString()); + } + finally{ + staticSynchLock.unlock(); } - if(loadError != null) throw new IOException(loadError.toString()); } public void start(Callback callback) throws IOException { @@ -49,34 +61,46 @@ JNotifyListener { if(new File(path).exists()) watches.add(JNotify.addWatch(path, mask, false, this)); } - synchronized(this) { - assert !started; - assert this.callback == null; - started = true; - this.callback = callback; - this.watches.addAll(watches); - } + synchLock.lock(); + try { + assert !started; + assert this.callback == null; + started = true; + this.callback = callback; + this.watches.addAll(watches); + } + finally{ + synchLock.unlock(); + } } public void stop() throws IOException { checkEnabled(); List<Integer> watches; - synchronized(this) { - assert started; - assert callback != null; - started = false; - callback = null; - watches = new ArrayList<Integer>(this.watches); - this.watches.clear(); - } + synchLock.lock(); + try { + assert started; + assert callback != null; + started = false; + callback = null; + watches = new ArrayList<Integer>(this.watches); + this.watches.clear(); + } + finally{ + synchLock.unlock(); + } for(Integer w : watches) JNotify.removeWatch(w); } public void fileCreated(int wd, String rootPath, String name) { Callback callback; - synchronized(this) { - callback = this.callback; - } + synchLock.lock(); + try { + callback = this.callback; + } + finally{ + synchLock.unlock(); + } if(callback != null) callback.driveInserted(new File(rootPath + "/" + name)); } diff --git a/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java b/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java index 08120a5a6e94caadcc77adf013b7f5d2cad1db6c..a8632dbd817fd5413ea656b93b4e22839342c6da 100644 --- a/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java +++ b/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java @@ -10,6 +10,10 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.concurrent.Executor; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Logger; import jssc.SerialPortEvent; @@ -42,8 +46,13 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { private int lineLen = 0; - private ReliabilityLayer reliability = null; // Locking: this - private boolean initialised = false, connected = false; // Locking: this + private ReliabilityLayer reliability = null; + private boolean initialised = false, connected = false; + + private final Lock synchLock = new ReentrantLock(); + private final Condition connectedStateChanged = synchLock.newCondition(); + private final Condition initialisedStateChanged = synchLock.newCondition(); + ModemImpl(Executor ioExecutor, ReliabilityLayerFactory reliabilityFactory, Clock clock, Callback callback, SerialPort port) { @@ -91,14 +100,18 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { // Wait for the event thread to receive "OK" boolean success = false; try { - synchronized(this) { + synchLock.lock(); + try { long now = clock.currentTimeMillis(); long end = now + OK_TIMEOUT; while(now < end && !initialised) { - wait(end - now); + initialisedStateChanged.await(end - now, TimeUnit.MILLISECONDS); now = clock.currentTimeMillis(); } success = initialised; + } + finally{ + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); @@ -123,11 +136,16 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { public void stop() throws IOException { LOG.info("Stopping"); - // Wake any threads that are waiting to connect - synchronized(this) { + synchLock.lock(); + try { + // Wake any threads that are waiting to connect initialised = false; connected = false; - notifyAll(); + initialisedStateChanged.signalAll(); + connectedStateChanged.signalAll(); + } + finally{ + synchLock.unlock(); } // Hang up if necessary and close the port try { @@ -145,10 +163,10 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { } } - // Locking: stateChange private void hangUpInner() throws IOException { ReliabilityLayer reliability; - synchronized(this) { + synchLock.lock(); + try { if(this.reliability == null) { LOG.info("Not hanging up - already on the hook"); return; @@ -156,6 +174,9 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { reliability = this.reliability; this.reliability = null; connected = false; + } + finally{ + synchLock.unlock(); } reliability.stop(); LOG.info("Hanging up"); @@ -182,7 +203,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { try { ReliabilityLayer reliability = reliabilityFactory.createReliabilityLayer(this); - synchronized(this) { + synchLock.lock(); + try { if(!initialised) { LOG.info("Not dialling - modem not initialised"); return false; @@ -192,6 +214,9 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { return false; } this.reliability = reliability; + } + finally{ + synchLock.unlock(); } reliability.start(); LOG.info("Dialling"); @@ -204,14 +229,18 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { } // Wait for the event thread to receive "CONNECT" try { - synchronized(this) { + synchLock.lock(); + try { long now = clock.currentTimeMillis(); long end = now + CONNECT_TIMEOUT; while(now < end && initialised && !connected) { - wait(end - now); + connectedStateChanged.await(end - now, TimeUnit.MILLISECONDS); now = clock.currentTimeMillis(); } if(connected) return true; + } + finally{ + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); @@ -227,8 +256,12 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { public InputStream getInputStream() throws IOException { ReliabilityLayer reliability; - synchronized(this) { + synchLock.lock(); + try { reliability = this.reliability; + } + finally{ + synchLock.unlock(); } if(reliability == null) throw new IOException("Not connected"); return reliability.getInputStream(); @@ -236,8 +269,12 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { public OutputStream getOutputStream() throws IOException { ReliabilityLayer reliability; - synchronized(this) { + synchLock.lock(); + try { reliability = this.reliability; + } + finally{ + synchLock.unlock(); } if(reliability == null) throw new IOException("Not connected"); return reliability.getOutputStream(); @@ -288,8 +325,12 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { private boolean handleData(byte[] b) throws IOException { ReliabilityLayer reliability; - synchronized(this) { + synchLock.lock(); + try { reliability = this.reliability; + } + finally{ + synchLock.unlock(); } if(reliability == null) return false; reliability.handleRead(b); @@ -309,9 +350,13 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { lineLen = 0; if(LOG.isLoggable(INFO)) LOG.info("Modem status: " + s); if(s.startsWith("CONNECT")) { - synchronized(this) { + synchLock.lock(); + try { connected = true; - notifyAll(); + connectedStateChanged.signalAll(); + } + finally{ + synchLock.unlock(); } // There might be data in the buffer as well as text int off = i + 1; @@ -323,14 +368,22 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { return; } else if(s.equals("BUSY") || s.equals("NO DIALTONE") || s.equals("NO CARRIER")) { - synchronized(this) { + synchLock.lock(); + try { connected = false; - notifyAll(); + connectedStateChanged.signalAll(); + } + finally{ + synchLock.unlock(); } } else if(s.equals("OK")) { - synchronized(this) { + synchLock.lock(); + try { initialised = true; - notifyAll(); + initialisedStateChanged.signalAll(); + } + finally{ + synchLock.unlock(); } } else if(s.equals("RING")) { ioExecutor.execute(new Runnable() { @@ -358,7 +411,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { try { ReliabilityLayer reliability = reliabilityFactory.createReliabilityLayer(this); - synchronized(this) { + synchLock.lock(); + try { if(!initialised) { LOG.info("Not answering - modem not initialised"); return; @@ -368,6 +422,9 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { return; } this.reliability = reliability; + } + finally{ + synchLock.unlock(); } reliability.start(); LOG.info("Answering"); @@ -380,14 +437,18 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { // Wait for the event thread to receive "CONNECT" boolean success = false; try { - synchronized(this) { + synchLock.lock(); + try { long now = clock.currentTimeMillis(); long end = now + CONNECT_TIMEOUT; while(now < end && initialised && !connected) { - wait(end - now); + connectedStateChanged.await(end - now, TimeUnit.MILLISECONDS); now = clock.currentTimeMillis(); } success = connected; + } + finally{ + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 1e61d1fd3a9bc5be8381127e4d6457cf08e7c473..5de946b072f1915182e99fe8c7a13a3ae44c1960 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=http\://services.gradle.org/distributions/gradle-1.12-all.zip +distributionUrl=http\://services.gradle.org/distributions/gradle-1.10-all.zip