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/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java b/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java index 597988cf90ce5f0a31958209de5a1a68866f640e..0f6bccfe6f2a493c3c23717335c832ca3136b048 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; @@ -47,6 +49,10 @@ Service, EventListener { private static final int PRIVATE_MESSAGE_NOTIFICATION_ID = 3; private static final int GROUP_POST_NOTIFICATION_ID = 4; + private static final String CONTACT_URI = + "content://org.briarproject/contact"; + private static final String GROUP_URI = + "content://org.briarproject/group"; private static final Logger LOG = Logger.getLogger(AndroidNotificationManagerImpl.class.getName()); @@ -55,13 +61,15 @@ Service, EventListener { private final Executor dbExecutor; private final EventBus eventBus; private final Context appContext; + private final Lock synchLock = new ReentrantLock(); + + // The following are locking: synchLock 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 - - private int privateTotal = 0, groupTotal = 0; // Locking: this - private int nextRequestId = 0; // Locking: this + new HashMap<GroupId, Integer>(); + private int privateTotal = 0, groupTotal = 0; + private int nextRequestId = 0; private volatile Settings settings = new Settings(); @@ -103,22 +111,32 @@ 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 + // Locking: synchLock private void updatePrivateMessageNotification() { if(privateTotal == 0) { clearPrivateMessageNotification(); @@ -143,6 +161,7 @@ Service, EventListener { Intent i = new Intent(appContext, ConversationActivity.class); ContactId c = contactCounts.keySet().iterator().next(); i.putExtra("briar.CONTACT_ID", c.getInt()); + i.setData(Uri.parse(CONTACT_URI + "/" + c.getInt())); i.setFlags(FLAG_ACTIVITY_CLEAR_TOP | FLAG_ACTIVITY_SINGLE_TOP); TaskStackBuilder t = TaskStackBuilder.create(appContext); t.addParentStack(ConversationActivity.class); @@ -162,7 +181,7 @@ Service, EventListener { } } - // Locking: this + // Locking: synchLock private void clearPrivateMessageNotification() { Object o = appContext.getSystemService(NOTIFICATION_SERVICE); NotificationManager nm = (NotificationManager) o; @@ -180,22 +199,32 @@ 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) { - Integer count = groupCounts.remove(g); - if(count == null) return; // Already cleared - groupTotal -= count; - updateGroupPostNotification(); + 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 + // Locking: synchLock private void updateGroupPostNotification() { if(groupTotal == 0) { clearGroupPostNotification(); @@ -219,6 +248,8 @@ Service, EventListener { Intent i = new Intent(appContext, GroupActivity.class); GroupId g = groupCounts.keySet().iterator().next(); i.putExtra("briar.GROUP_ID", g.getBytes()); + String idHex = StringUtils.toHexString(g.getBytes()); + i.setData(Uri.parse(GROUP_URI + "/" + idHex)); i.setFlags(FLAG_ACTIVITY_CLEAR_TOP | FLAG_ACTIVITY_SINGLE_TOP); TaskStackBuilder t = TaskStackBuilder.create(appContext); t.addParentStack(GroupActivity.class); @@ -238,18 +269,23 @@ Service, EventListener { } } - // Locking: this + // Locking: synchLock 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..b7b764f097e6f8fde530543bd706eb6ed2e53fa2 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,67 @@ class ReferenceManagerImpl implements ReferenceManager { private static final Logger LOG = Logger.getLogger(ReferenceManagerImpl.class.getName()); - // Locking: this + private final Lock synchLock = new ReentrantLock(); + + // The following are locking: synchLock private final Map<Class<?>, Map<Long, Object>> outerMap = new HashMap<Class<?>, Map<Long, Object>>(); + private long nextHandle = 0; - private long nextHandle = 0; // Locking: this - - public synchronized <T> T getReference(long handle, Class<T> c) { - Map<Long, Object> innerMap = outerMap.get(c); - if(innerMap == null) { + 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; + } finally { + synchLock.unlock(); } - long handle = nextHandle++; - innerMap.put(handle, reference); - if(LOG.isLoggable(INFO)) { - LOG.info(innerMap.size() + " handles for " + c.getName() + - " after put"); - } - 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); + } finally { + synchLock.unlock(); } - return c.cast(o); + } } diff --git a/briar-android/src/org/briarproject/system/AndroidLocationUtils.java b/briar-android/src/org/briarproject/system/AndroidLocationUtils.java index 6a104e4d522b04832599397b3bfb9d63a8e69c2d..a73c9233bf3eb78b0fc0189cf2fea4444eb70164 100644 --- a/briar-android/src/org/briarproject/system/AndroidLocationUtils.java +++ b/briar-android/src/org/briarproject/system/AndroidLocationUtils.java @@ -34,9 +34,6 @@ class AndroidLocationUtils implements LocationUtils { * <ul> * <li>Phone network. This works even when no SIM card is inserted, or a * foreign SIM card is inserted.</li> - * <li><del>Location service (GPS/WiFi/etc).</del> <em>This is disabled for - * now, until we figure out an offline method of converting a long/lat - * into a country code, that doesn't involve a network call.</em> * <li>SIM card. This is only an heuristic and assumes the user is not * roaming.</li> * <li>User locale. This is an even worse heuristic.</li> diff --git a/briar-core/src/org/briarproject/crypto/FortunaGenerator.java b/briar-core/src/org/briarproject/crypto/FortunaGenerator.java index 48cf66e5fddfda2fdef5560d435cf9ae62246aa9..4045db3ce19742473aa65f6dc175c6abc535b1b1 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,7 +19,9 @@ 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 Lock synchLock = new ReentrantLock(); + + // The following are locking: synchLock private final MessageDigest digest = new DoubleDigest(new SHA256Digest()); private final BlockCipher cipher = new AESLightEngine(); private final byte[] key = new byte[KEY_BYTES]; @@ -28,56 +33,78 @@ class FortunaGenerator { 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(); - } - // 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(); + 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; + } 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 b29dc806d2abeb85355a27c9819fac6442ce1204..9987062acac077eb24cf08953c4328422beb7471 100644 --- a/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java +++ b/briar-core/src/org/briarproject/crypto/PseudoRandomImpl.java @@ -14,7 +14,7 @@ class PseudoRandomImpl implements PseudoRandom { generator = new FortunaGenerator(seed); } - public synchronized byte[] nextBytes(int length) { + public byte[] nextBytes(int length) { byte[] b = new byte[length]; int offset = 0; while(offset < length) offset += generator.nextBytes(b, offset, length); diff --git a/briar-core/src/org/briarproject/db/JdbcDatabase.java b/briar-core/src/org/briarproject/db/JdbcDatabase.java index aaa8aee814ef22b2762df7a0863ec1e95134d9a1..4b45adcdc49301aab0acdf01db5dadc493e8b880 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,16 +316,19 @@ abstract class JdbcDatabase implements Database<Connection> { private final Clock clock; private final LinkedList<Connection> connections = - new LinkedList<Connection>(); // Locking: self + new LinkedList<Connection>(); // Locking: connectionsLock private final AtomicInteger transactionCount = new AtomicInteger(0); - private int openConnections = 0; // Locking: connections - private boolean closed = false; // Locking: connections + private int openConnections = 0; // Locking: connectionsLock + private boolean closed = false; // Locking: connectionsLock 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) { this.hashType = hashType; @@ -431,9 +437,12 @@ 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) { @@ -441,8 +450,11 @@ abstract class JdbcDatabase implements Database<Connection> { txn = createConnection(); if(txn == null) throw new DbException(); txn.setAutoCommit(false); - synchronized(connections) { + connectionsLock.lock(); + try { openConnections++; + } finally { + connectionsLock.unlock(); } } } catch(SQLException e) { @@ -455,9 +467,12 @@ 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,9 +483,12 @@ 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(); } } } @@ -486,9 +504,12 @@ 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 +523,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; @@ -518,7 +540,10 @@ abstract class JdbcDatabase implements Database<Connection> { openConnections -= connections.size(); 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..97bd39ddaa60dc5637a5852d2ba55c159348025d 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,13 +62,9 @@ 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 - * calling listeners.add() must be guarded by a lock. - */ + // The following are locking: synchLock private int localConfirmationCode = -1, remoteConfirmationCode = -1; private boolean connectionFailed = false; private boolean localCompared = false, remoteCompared = false; @@ -104,12 +102,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,8 +134,11 @@ 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,8 +170,11 @@ 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,17 +203,23 @@ 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,9 +232,12 @@ 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,31 +249,43 @@ 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..880db5b5eb497fbf256899a1a05296bd2dd2f7c3 100644 --- a/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java +++ b/briar-core/src/org/briarproject/lifecycle/ShutdownManagerImpl.java @@ -2,34 +2,50 @@ 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 + private final Lock synchLock = new ReentrantLock(); - private int nextHandle = 0; // Locking: this + // The following are locking: synchLock + protected final Map<Integer, Thread> hooks; + private int nextHandle = 0; 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/messaging/MessagingModule.java b/briar-core/src/org/briarproject/messaging/MessagingModule.java index 5986266b8dd50b2062b0499e2b31f18d04833926..e99c2a42c309ba5eb2c828db3fec19f6cf562fff 100644 --- a/briar-core/src/org/briarproject/messaging/MessagingModule.java +++ b/briar-core/src/org/briarproject/messaging/MessagingModule.java @@ -9,9 +9,9 @@ import org.briarproject.api.messaging.Group; import org.briarproject.api.messaging.GroupFactory; import org.briarproject.api.messaging.MessageFactory; import org.briarproject.api.messaging.MessageVerifier; +import org.briarproject.api.messaging.MessagingSessionFactory; import org.briarproject.api.messaging.PacketReaderFactory; import org.briarproject.api.messaging.PacketWriterFactory; -import org.briarproject.api.messaging.MessagingSessionFactory; import org.briarproject.api.messaging.SubscriptionUpdate; import org.briarproject.api.messaging.UnverifiedMessage; import org.briarproject.api.serial.StructReader; diff --git a/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java b/briar-core/src/org/briarproject/plugins/ConnectionRegistryImpl.java index 2f7d0788968d34e25146237f630a35af58326995..358f6da43a300345684bd1c33d81a9fd16f1d4af 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; @@ -25,9 +27,10 @@ class ConnectionRegistryImpl implements ConnectionRegistry { Logger.getLogger(ConnectionRegistryImpl.class.getName()); private final EventBus eventBus; - // Locking: this + private final Lock synchLock = new ReentrantLock(); + + // The following are locking: synchLock private final Map<TransportId, Map<ContactId, Integer>> connections; - // Locking: this private final Map<ContactId, Integer> contactCounts; @Inject @@ -40,7 +43,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>(); @@ -56,7 +60,10 @@ class ConnectionRegistryImpl implements ConnectionRegistry { } else { contactCounts.put(c, count + 1); } + } finally { + synchLock.unlock(); } + if(firstConnection) { LOG.info("Contact connected"); eventBus.broadcast(new ContactConnectedEvent(c)); @@ -66,7 +73,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); @@ -84,23 +92,38 @@ class ConnectionRegistryImpl implements ConnectionRegistry { } else { 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..b45bc274845cb538468c67f9cd6007918cd9a0fd 100644 --- a/briar-core/src/org/briarproject/reliability/Receiver.java +++ b/briar-core/src/org/briarproject/reliability/Receiver.java @@ -1,10 +1,15 @@ package org.briarproject.reliability; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + import java.io.IOException; import java.util.Comparator; import java.util.Iterator; import java.util.SortedSet; import java.util.TreeSet; +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,9 +21,13 @@ class Receiver implements ReadHandler { private final Clock clock; private final Sender sender; - private final SortedSet<Data> dataFrames; // Locking: this + private final Lock windowLock = new ReentrantLock(); + private final Condition dataFrameAvailable = windowLock.newCondition(); + + // The following are locking: windowLock + private final SortedSet<Data> dataFrames; + private int windowSize = MAX_WINDOW_SIZE; - private int windowSize = MAX_WINDOW_SIZE; // Locking: this private long finalSequenceNumber = Long.MAX_VALUE; private long nextSequenceNumber = 1; @@ -30,36 +39,44 @@ 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 { + windowLock.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, 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, MILLISECONDS); + } } + now = clock.currentTimeMillis(); } - now = clock.currentTimeMillis(); + if(valid) throw new IOException("Read timed out"); + throw new IOException("Connection closed"); + } finally { + windowLock.unlock(); } - if(valid) throw new IOException("Read timed out"); - throw new IOException("Connection closed"); } void invalidate() { valid = false; - synchronized(this) { - notifyAll(); + windowLock.lock(); + try { + dataFrameAvailable.signalAll(); + } finally { + windowLock.unlock(); } } @@ -79,43 +96,48 @@ 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 { + windowLock.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 { + windowLock.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..6cb5d6d8f4f47998f3fd375f40d4a8f50e2f54b7 100644 --- a/briar-core/src/org/briarproject/reliability/Sender.java +++ b/briar-core/src/org/briarproject/reliability/Sender.java @@ -1,10 +1,15 @@ package org.briarproject.reliability; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + import java.io.IOException; import java.util.ArrayList; import java.util.Iterator; import java.util.LinkedList; import java.util.List; +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 +26,11 @@ class Sender { private final Clock clock; private final WriteHandler writeHandler; - private final LinkedList<Outstanding> outstanding; // Locking: this + private final Lock windowLock = new ReentrantLock(); + private final Condition sendWindowAvailable = windowLock.newCondition(); - // All of the following are locking: this + // The following are locking: windowLock + private final LinkedList<Outstanding> outstanding; private int outstandingBytes = 0; private int windowSize = Data.MAX_PAYLOAD_LENGTH; private int rtt = INITIAL_RTT, rttVar = INITIAL_RTT_VAR; @@ -58,7 +65,8 @@ class Sender { long sequenceNumber = a.getSequenceNumber(); long now = clock.currentTimeMillis(); Outstanding fastRetransmit = null; - synchronized(this) { + windowLock.lock(); + try { // Remove the acked data frame if it's outstanding int foundIndex = -1; Iterator<Outstanding> it = outstanding.iterator(); @@ -94,7 +102,10 @@ class Sender { // Don't accept an unreasonably large window size windowSize = Math.min(a.getWindowSize(), MAX_WINDOW_SIZE); // If space has become available, notify any waiting writers - if(windowSize > oldWindowSize || foundIndex != -1) notifyAll(); + if(windowSize > oldWindowSize || foundIndex != -1) + sendWindowAvailable.signalAll(); + } finally { + windowLock.unlock(); } // Fast retransmission if(fastRetransmit != null) @@ -105,7 +116,8 @@ class Sender { long now = clock.currentTimeMillis(); List<Outstanding> retransmit = null; boolean sendProbe = false; - synchronized(this) { + windowLock.lock(); + try { if(outstanding.isEmpty()) { if(dataWaiting && now - lastWindowUpdateOrProbe > rto) { sendProbe = true; @@ -134,6 +146,8 @@ class Sender { } } } + } finally { + windowLock.unlock(); } // Send a window probe if necessary if(sendProbe) { @@ -151,12 +165,13 @@ class Sender { void write(Data d) throws IOException, InterruptedException { int payloadLength = d.getPayloadLength(); - synchronized(this) { + windowLock.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, MILLISECONDS); now = clock.currentTimeMillis(); } if(outstandingBytes + payloadLength >= windowSize) @@ -164,12 +179,20 @@ class Sender { outstanding.add(new Outstanding(d, now)); outstandingBytes += payloadLength; dataWaiting = false; + } finally { + windowLock.unlock(); } writeHandler.handleWrite(d.getBuffer()); } - synchronized void flush() throws IOException, InterruptedException { - while(dataWaiting || !outstanding.isEmpty()) wait(); + void flush() throws IOException, InterruptedException { + windowLock.lock(); + try { + while(dataWaiting || !outstanding.isEmpty()) + sendWindowAvailable.await(); + } finally { + windowLock.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 089f724f8605a6d2043f1e26e6d52a7bedd2c481..257b1ef619af3e81bc5768684951569970872746 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; @@ -48,8 +50,9 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { private final TagRecogniser tagRecogniser; private final Clock clock; private final Timer timer; + private final Lock synchLock = new ReentrantLock(); - // All of the following are locking: this + // The following are locking: synchLock private final Map<TransportId, Integer> maxLatencies; private final Map<EndpointKey, TemporarySecret> oldSecrets; private final Map<EndpointKey, TemporarySecret> currentSecrets; @@ -71,45 +74,54 @@ 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 DB + 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 + // Locking: synchLock private Collection<TemporarySecret> assignSecretsToMaps(long now, Collection<TemporarySecret> secrets) { Collection<TemporarySecret> dead = new ArrayList<TemporarySecret>(); @@ -142,7 +154,7 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } // Replaces the given secrets and returns any secrets created - // Locking: this + // Locking: synchLock private Collection<TemporarySecret> replaceDeadSecrets(long now, Collection<TemporarySecret> dead) { // If there are several dead secrets for an endpoint, use the newest @@ -200,105 +212,125 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { return created; } - public synchronized boolean stop() { - eventBus.removeListener(this); - timer.cancel(); - tagRecogniser.removeSecrets(); - maxLatencies.clear(); - oldSecrets.clear(); - currentSecrets.clear(); - newSecrets.clear(); - return true; + public boolean stop() { + synchLock.lock(); + try { + eventBus.removeListener(this); + timer.cancel(); + tagRecogniser.removeSecrets(); + maxLatencies.clear(); + oldSecrets.clear(); + currentSecrets.clear(); + newSecrets.clear(); + return true; + } finally { + synchLock.unlock(); + } } - 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; + synchLock.lock(); try { - streamNumber = db.incrementStreamCounter(c, t, s.getPeriod()); - if(streamNumber == -1) { - LOG.info("No counter for period"); + TemporarySecret s = currentSecrets.get(new EndpointKey(c, t)); + if(s == null) { + LOG.info("No secret for endpoint"); return null; } - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - 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; + } + byte[] secret = s.getSecret(); + return new StreamContext(c, t, secret, streamNumber, s.getAlice()); + } finally { + synchLock.unlock(); } - byte[] secret = s.getSecret(); - return new StreamContext(c, t, secret, streamNumber, s.getAlice()); } - public synchronized void endpointAdded(Endpoint ep, int maxLatency, + public void endpointAdded(Endpoint ep, int 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++) - b1 = crypto.deriveNextSecret(b1, p); - 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 + synchLock.lock(); try { - db.addSecrets(Arrays.asList(s1, s2, s3)); - } catch(DbException e) { - if(LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e); - return; + 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++) + b1 = crypto.deriveNextSecret(b1, p); + 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); + } finally { + synchLock.unlock(); } - // Pass the new secrets to the recogniser - tagRecogniser.addSecret(s1); - tagRecogniser.addSecret(s2); - tagRecogniser.addSecret(s3); } @Override - public synchronized void run() { - // 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); + 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); } - // 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(); } } @@ -315,14 +347,14 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { } } - // Locking: this + // Locking: synchLock private void removeSecrets(ContactId c, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); while(it.hasNext()) if(it.next().getContactId().equals(c)) it.remove(); } - // Locking: this + // Locking: synchLock private void removeSecrets(TransportId t, Map<?, TemporarySecret> m) { Iterator<TemporarySecret> it = m.values().iterator(); while(it.hasNext()) @@ -371,10 +403,13 @@ class KeyManagerImpl extends TimerTask implements KeyManager, EventListener { public void run() { ContactId c = event.getContactId(); tagRecogniser.removeSecrets(c); - synchronized(KeyManagerImpl.this) { + synchLock.lock(); + try { removeSecrets(c, oldSecrets); removeSecrets(c, currentSecrets); removeSecrets(c, newSecrets); + } finally { + synchLock.unlock(); } } } @@ -389,8 +424,11 @@ 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(); } } } @@ -407,11 +445,14 @@ 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); removeSecrets(t, oldSecrets); removeSecrets(t, currentSecrets); removeSecrets(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..ea4e5567b5a1e22c464056cbeec373d6f09c64f1 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,7 +20,9 @@ class TagRecogniserImpl implements TagRecogniser { private final CryptoComponent crypto; private final DatabaseComponent db; - // Locking: this + private final Lock synchLock = new ReentrantLock(); + + // Locking: synchLock private final Map<TransportId, TransportTagRecogniser> recognisers; @Inject @@ -31,8 +35,11 @@ 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 +48,58 @@ 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 285b4465e4543fdd5a1bb252b6d5b8c77ed02c5f..08981204404bd9fa0ceb223e2d51cf8dcb3fa11f 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,11 @@ 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 Lock synchLock = new ReentrantLock(); + + // The following are locking: synchLock + private final Map<Bytes, TagContext> tagMap; + private final Map<RemovalKey, RemovalContext> removalMap; TransportTagRecogniser(CryptoComponent crypto, DatabaseComponent db, TransportId transportId) { @@ -39,61 +44,76 @@ 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; + } } + // Store the updated reordering window in the DB + db.setReorderingWindow(t.contactId, transportId, t.period, + t.window.getCentre(), t.window.getBitmap()); + return new StreamContext(t.contactId, transportId, t.secret, + t.streamNumber, t.alice); + } finally { + synchLock.unlock(); } - // Store the updated reordering window in the DB - db.setReorderingWindow(t.contactId, transportId, t.period, - t.window.getCentre(), t.window.getBitmap()); - return new StreamContext(t.contactId, transportId, t.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; + } + // 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(); } - // 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 + // Locking: synchLock private void removeSecret(RemovalContext r) { // Remove the expected tags SecretKey key = crypto.deriveTagKey(r.secret, !r.alice); @@ -105,17 +125,28 @@ class TransportTagRecogniser { } } - 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-core/src/org/briarproject/util/LatchedReference.java b/briar-core/src/org/briarproject/util/LatchedReference.java index d43ace1edf85314e91f660bd3918d19ebceb2fdf..c9f35b687c6264accebc787b49e6de2c92a6ca5e 100644 --- a/briar-core/src/org/briarproject/util/LatchedReference.java +++ b/briar-core/src/org/briarproject/util/LatchedReference.java @@ -1,7 +1,8 @@ package org.briarproject.util; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; public class LatchedReference<T> { @@ -23,7 +24,7 @@ public class LatchedReference<T> { } public T waitForReference(long timeout) throws InterruptedException { - latch.await(timeout, TimeUnit.MILLISECONDS); + latch.await(timeout, MILLISECONDS); return reference.get(); } } diff --git a/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java b/briar-desktop/src/org/briarproject/lifecycle/WindowsShutdownManagerImpl.java index 5b1715cfa559154fe698d77373fb1bc5455f2f7a..3585e557f2941e8e9aa6b55ea6452a29e361e267 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; @@ -36,8 +38,9 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { private static final int WS_MINIMIZE = 0x20000000; private final Map<String, Object> options; + private final Lock synchLock = new ReentrantLock(); - private boolean initialised = false; // Locking: this + private boolean initialised = false; // Locking: synchLock WindowsShutdownManagerImpl() { // Use the Unicode versions of Win32 API calls @@ -48,9 +51,14 @@ 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 +66,7 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { return new StartOnce(r); } - // Locking: this + // Locking: synchLock private void initialise() { if(OsUtils.isWindows()) { new EventLoop().start(); @@ -69,20 +77,25 @@ 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 { @@ -92,7 +105,7 @@ class WindowsShutdownManagerImpl extends ShutdownManagerImpl { } @Override - public void run() { + public void run() { try { // Load user32.dll final User32 user32 = (User32) Native.loadLibrary("user32", diff --git a/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java b/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java index a2564906534d73d207a82bd5d27608fc751bcb81..c86b657d7ed3fb3c2fd99a8ff4c03ab0bfd39742 100644 --- a/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java +++ b/briar-desktop/src/org/briarproject/plugins/file/PollingRemovableDriveMonitor.java @@ -1,9 +1,14 @@ package org.briarproject.plugins.file; +import static java.util.concurrent.TimeUnit.MILLISECONDS; + import java.io.File; import java.io.IOException; import java.util.Collection; import java.util.concurrent.Executor; +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,7 +19,9 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { private final Executor ioExecutor; private final RemovableDriveFinder finder; private final int pollingInterval; - private final Object pollingLock = new Object(); + + private final Lock pollingLock = new ReentrantLock(); + private final Condition stopPolling = pollingLock.newCondition(); private volatile boolean running = false; private volatile Callback callback = null; @@ -34,8 +41,12 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { public void stop() throws IOException { running = false; - synchronized(pollingLock) { - pollingLock.notifyAll(); + pollingLock.lock(); + try { + stopPolling.signalAll(); + } + finally { + pollingLock.unlock(); } } @@ -43,8 +54,11 @@ class PollingRemovableDriveMonitor implements RemovableDriveMonitor, Runnable { try { Collection<File> drives = finder.findRemovableDrives(); while(running) { - synchronized(pollingLock) { - pollingLock.wait(pollingInterval); + pollingLock.lock(); + try { + stopPolling.await(pollingInterval, MILLISECONDS); + } finally { + pollingLock.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..bc2ca3fd9fc29883027c547941890441f5de64ef 100644 --- a/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java +++ b/briar-desktop/src/org/briarproject/plugins/file/UnixRemovableDriveMonitor.java @@ -4,6 +4,8 @@ import java.io.File; import java.io.IOException; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import net.contentobjects.jnotify.JNotify; import net.contentobjects.jnotify.JNotifyListener; @@ -11,14 +13,19 @@ 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 + //TODO: rationalise this in a further refactor + private static final Lock staticSynchLock = new ReentrantLock(); - // Locking: this - private final List<Integer> watches = new ArrayList<Integer>(); + // The following are locking: staticSynchLock + private static boolean triedLoad = false; + private static Throwable loadError = null; + + private final Lock synchLock = new ReentrantLock(); - private boolean started = false; // Locking: this - private Callback callback = null; // Locking: this + // The following are locking: synchLock + private final List<Integer> watches = new ArrayList<Integer>(); + private boolean started = false; + private Callback callback = null; protected abstract String[] getPathsToWatch(); @@ -33,12 +40,17 @@ 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,33 +61,42 @@ JNotifyListener { if(new File(path).exists()) watches.add(JNotify.addWatch(path, mask, false, this)); } - synchronized(this) { + 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) { + 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) { + 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..2896f48d02511310f36cd5bf43c02082d7b546e6 100644 --- a/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java +++ b/briar-desktop/src/org/briarproject/plugins/modem/ModemImpl.java @@ -1,5 +1,6 @@ package org.briarproject.plugins.modem; +import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.logging.Level.INFO; import static java.util.logging.Level.WARNING; import static jssc.SerialPort.PURGE_RXCLEAR; @@ -10,6 +11,9 @@ import java.io.InputStream; import java.io.OutputStream; import java.util.concurrent.Executor; import java.util.concurrent.Semaphore; +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; @@ -40,10 +44,15 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { private final Semaphore stateChange; private final byte[] line; - private int lineLen = 0; + private final Lock synchLock = new ReentrantLock(); + private final Condition connectedStateChanged = synchLock.newCondition(); + private final Condition initialisedStateChanged = synchLock.newCondition(); + + // The following are locking: synchLock + private ReliabilityLayer reliability = null; + private boolean initialised = false, connected = false; - private ReliabilityLayer reliability = null; // Locking: this - private boolean initialised = false, connected = false; // Locking: this + private int lineLen = 0; ModemImpl(Executor ioExecutor, ReliabilityLayerFactory reliabilityFactory, Clock clock, Callback callback, SerialPort port) { @@ -91,14 +100,17 @@ 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, MILLISECONDS); now = clock.currentTimeMillis(); } success = initialised; + } finally { + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); @@ -123,11 +135,15 @@ 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 { @@ -148,7 +164,8 @@ 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 +173,8 @@ 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 +201,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 +212,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { return false; } this.reliability = reliability; + } finally { + synchLock.unlock(); } reliability.start(); LOG.info("Dialling"); @@ -204,14 +226,17 @@ 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, MILLISECONDS); now = clock.currentTimeMillis(); } if(connected) return true; + } finally { + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); @@ -227,8 +252,11 @@ 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 +264,11 @@ 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 +319,11 @@ 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 +343,12 @@ 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 +360,20 @@ 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 +401,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 +412,8 @@ class ModemImpl implements Modem, WriteHandler, SerialPortEventListener { return; } this.reliability = reliability; + } finally { + synchLock.unlock(); } reliability.start(); LOG.info("Answering"); @@ -380,14 +426,17 @@ 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, MILLISECONDS); now = clock.currentTimeMillis(); } success = connected; + } finally { + synchLock.unlock(); } } catch(InterruptedException e) { tryToClose(port); diff --git a/briar-tests/src/org/briarproject/crypto/StreamEncrypterImplTest.java b/briar-tests/src/org/briarproject/crypto/StreamEncrypterImplTest.java index 922d1fa17c22655d20c97535f939bb8453a3e4f3..44c61fdee6096f367ecc8e3ac37ae11f538adf9b 100644 --- a/briar-tests/src/org/briarproject/crypto/StreamEncrypterImplTest.java +++ b/briar-tests/src/org/briarproject/crypto/StreamEncrypterImplTest.java @@ -38,8 +38,8 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, false, payloadLength, 0); - byte[] expected = new byte[TAG_LENGTH + HEADER_LENGTH + payloadLength - + MAC_LENGTH]; + int frameLength = HEADER_LENGTH + payloadLength + MAC_LENGTH; + byte[] expected = new byte[TAG_LENGTH + frameLength]; System.arraycopy(tag, 0, expected, 0, TAG_LENGTH); System.arraycopy(header, 0, expected, TAG_LENGTH, HEADER_LENGTH); System.arraycopy(payload, 0, expected, TAG_LENGTH + HEADER_LENGTH, @@ -53,6 +53,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, tag); int payloadLength = 123; + int frameLength = HEADER_LENGTH + payloadLength + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -60,8 +61,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, true, payloadLength, 0); - byte[] expected = new byte[TAG_LENGTH + HEADER_LENGTH + payloadLength - + MAC_LENGTH]; + byte[] expected = new byte[TAG_LENGTH + frameLength]; System.arraycopy(tag, 0, expected, 0, TAG_LENGTH); System.arraycopy(header, 0, expected, TAG_LENGTH, HEADER_LENGTH); System.arraycopy(payload, 0, expected, TAG_LENGTH + HEADER_LENGTH, @@ -75,6 +75,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, null); int payloadLength = 123; + int frameLength = HEADER_LENGTH + payloadLength + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -82,7 +83,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, false, payloadLength, 0); - byte[] expected = new byte[HEADER_LENGTH + payloadLength + MAC_LENGTH]; + byte[] expected = new byte[frameLength]; System.arraycopy(header, 0, expected, 0, HEADER_LENGTH); System.arraycopy(payload, 0, expected, HEADER_LENGTH, payloadLength); assertArrayEquals(expected, out.toByteArray()); @@ -94,6 +95,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, null); int payloadLength = 123; + int frameLength = HEADER_LENGTH + payloadLength + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -101,7 +103,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, true, payloadLength, 0); - byte[] expected = new byte[HEADER_LENGTH + payloadLength + MAC_LENGTH]; + byte[] expected = new byte[frameLength]; System.arraycopy(header, 0, expected, 0, HEADER_LENGTH); System.arraycopy(payload, 0, expected, HEADER_LENGTH, payloadLength); assertArrayEquals(expected, out.toByteArray()); @@ -113,6 +115,8 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, tag); int payloadLength = 123, paddingLength = 234; + int frameLength = HEADER_LENGTH + payloadLength + paddingLength + + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -120,8 +124,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, false, payloadLength, paddingLength); - byte[] expected = new byte[TAG_LENGTH + HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH]; + byte[] expected = new byte[TAG_LENGTH + frameLength]; System.arraycopy(tag, 0, expected, 0, TAG_LENGTH); System.arraycopy(header, 0, expected, TAG_LENGTH, HEADER_LENGTH); System.arraycopy(payload, 0, expected, TAG_LENGTH + HEADER_LENGTH, @@ -135,6 +138,8 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, tag); int payloadLength = 123, paddingLength = 234; + int frameLength = HEADER_LENGTH + payloadLength + paddingLength + + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -142,8 +147,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, true, payloadLength, paddingLength); - byte[] expected = new byte[TAG_LENGTH + HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH]; + byte[] expected = new byte[TAG_LENGTH + frameLength]; System.arraycopy(tag, 0, expected, 0, TAG_LENGTH); System.arraycopy(header, 0, expected, TAG_LENGTH, HEADER_LENGTH); System.arraycopy(payload, 0, expected, TAG_LENGTH + HEADER_LENGTH, @@ -157,6 +161,8 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, null); int payloadLength = 123, paddingLength = 234; + int frameLength = HEADER_LENGTH + payloadLength + paddingLength + + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -164,8 +170,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, false, payloadLength, paddingLength); - byte[] expected = new byte[HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH]; + byte[] expected = new byte[frameLength]; System.arraycopy(header, 0, expected, 0, HEADER_LENGTH); System.arraycopy(payload, 0, expected, HEADER_LENGTH, payloadLength); assertArrayEquals(expected, out.toByteArray()); @@ -177,6 +182,8 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, null); int payloadLength = 123, paddingLength = 234; + int frameLength = HEADER_LENGTH + payloadLength + paddingLength + + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); @@ -184,8 +191,7 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header, true, payloadLength, paddingLength); - byte[] expected = new byte[HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH]; + byte[] expected = new byte[frameLength]; System.arraycopy(header, 0, expected, 0, HEADER_LENGTH); System.arraycopy(payload, 0, expected, HEADER_LENGTH, payloadLength); assertArrayEquals(expected, out.toByteArray()); @@ -197,9 +203,13 @@ public class StreamEncrypterImplTest extends BriarTestCase { StreamEncrypterImpl s = new StreamEncrypterImpl(out, frameCipher, frameKey, null); int payloadLength = 123, paddingLength = 234; + int frameLength = HEADER_LENGTH + payloadLength + paddingLength + + MAC_LENGTH; byte[] payload = new byte[payloadLength]; new Random().nextBytes(payload); int payloadLength1 = 345, paddingLength1 = 456; + int frameLength1 = HEADER_LENGTH + payloadLength1 + paddingLength1 + + MAC_LENGTH; byte[] payload1 = new byte[payloadLength1]; new Random().nextBytes(payload1); @@ -211,16 +221,12 @@ public class StreamEncrypterImplTest extends BriarTestCase { byte[] header1 = new byte[HEADER_LENGTH]; FrameEncoder.encodeHeader(header1, true, payloadLength1, paddingLength1); - byte[] expected = new byte[HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH - + HEADER_LENGTH + payloadLength1 - + paddingLength1 + MAC_LENGTH]; + byte[] expected = new byte[frameLength + frameLength1]; System.arraycopy(header, 0, expected, 0, HEADER_LENGTH); System.arraycopy(payload, 0, expected, HEADER_LENGTH, payloadLength); - System.arraycopy(header1, 0, expected, HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH, HEADER_LENGTH); - System.arraycopy(payload1, 0, expected, HEADER_LENGTH + payloadLength - + paddingLength + MAC_LENGTH + HEADER_LENGTH, payloadLength1); + System.arraycopy(header1, 0, expected, frameLength, HEADER_LENGTH); + System.arraycopy(payload1, 0, expected, frameLength + HEADER_LENGTH, + payloadLength1); assertArrayEquals(expected, out.toByteArray()); } diff --git a/briar-tests/src/org/briarproject/plugins/ConnectionRegistryImplTest.java b/briar-tests/src/org/briarproject/plugins/ConnectionRegistryImplTest.java index 08fd4942c2745a4749edec07195f8a844ed94ac1..045a774f1bc3b7acb28e8a135f0811d4706adfcb 100644 --- a/briar-tests/src/org/briarproject/plugins/ConnectionRegistryImplTest.java +++ b/briar-tests/src/org/briarproject/plugins/ConnectionRegistryImplTest.java @@ -11,7 +11,6 @@ import org.briarproject.api.event.ContactConnectedEvent; import org.briarproject.api.event.ContactDisconnectedEvent; import org.briarproject.api.event.EventBus; import org.briarproject.api.plugins.ConnectionRegistry; -import org.briarproject.plugins.ConnectionRegistryImpl; import org.jmock.Expectations; import org.jmock.Mockery; import org.junit.Test;