From 07b34cfbab867621b1a41ec84e9c4867a5f43736 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Thu, 11 Aug 2011 15:19:32 +0100
Subject: [PATCH] Added a lock for the connectionWindows table and exposed
getConnectionWindow() and setConnectionWindow() through the DatabaseComponent
interface.
---
.../sf/briar/api/db/DatabaseComponent.java | 15 ++++++
.../api/transport/ConnectionRecogniser.java | 3 +-
components/net/sf/briar/db/Database.java | 9 +++-
components/net/sf/briar/db/JdbcDatabase.java | 12 ++---
.../db/ReadWriteLockDatabaseComponent.java | 50 +++++++++++++++++++
.../db/SynchronizedDatabaseComponent.java | 37 ++++++++++++++
.../sf/briar/invitation/InvitationWorker.java | 9 ++--
.../sf/briar/db/DatabaseComponentTest.java | 33 ++++++++++--
.../transport/ConnectionWindowImplTest.java | 9 ++++
9 files changed, 159 insertions(+), 18 deletions(-)
diff --git a/api/net/sf/briar/api/db/DatabaseComponent.java b/api/net/sf/briar/api/db/DatabaseComponent.java
index 2ef2b4d96a..03188adeb6 100644
--- a/api/net/sf/briar/api/db/DatabaseComponent.java
+++ b/api/net/sf/briar/api/db/DatabaseComponent.java
@@ -22,6 +22,7 @@ import net.sf.briar.api.protocol.writers.OfferWriter;
import net.sf.briar.api.protocol.writers.RequestWriter;
import net.sf.briar.api.protocol.writers.SubscriptionWriter;
import net.sf.briar.api.protocol.writers.TransportWriter;
+import net.sf.briar.api.transport.ConnectionWindow;
/**
* Encapsulates the database implementation and exposes high-level operations
@@ -103,6 +104,13 @@ public interface DatabaseComponent {
void generateTransportUpdate(ContactId c, TransportWriter t) throws
DbException, IOException;
+ /**
+ * Returns the connection reordering window for the given contact and
+ * transport.
+ */
+ ConnectionWindow getConnectionWindow(ContactId c, int transportId)
+ throws DbException;
+
/** Returns the IDs of all contacts. */
Collection<ContactId> getContacts() throws DbException;
@@ -156,6 +164,13 @@ public interface DatabaseComponent {
/** Removes a contact (and all associated state) from the database. */
void removeContact(ContactId c) throws DbException;
+ /**
+ * Sets the connection reordering window for the given contact and
+ * transport.
+ */
+ void setConnectionWindow(ContactId c, int transportId, ConnectionWindow w)
+ throws DbException;
+
/** Records the user's rating for the given author. */
void setRating(AuthorId a, Rating r) throws DbException;
diff --git a/api/net/sf/briar/api/transport/ConnectionRecogniser.java b/api/net/sf/briar/api/transport/ConnectionRecogniser.java
index 3ec801404f..511f65cf57 100644
--- a/api/net/sf/briar/api/transport/ConnectionRecogniser.java
+++ b/api/net/sf/briar/api/transport/ConnectionRecogniser.java
@@ -1,6 +1,7 @@
package net.sf.briar.api.transport;
import net.sf.briar.api.ContactId;
+import net.sf.briar.api.db.DbException;
/**
* Maintains a transport plugin's connection reordering window and decides
@@ -12,5 +13,5 @@ public interface ConnectionRecogniser {
* Returns the ID of the contact who created the tag if the connection
* should be accepted, or null if the connection should be rejected.
*/
- ContactId acceptConnection(byte[] tag);
+ ContactId acceptConnection(byte[] tag) throws DbException;
}
diff --git a/components/net/sf/briar/db/Database.java b/components/net/sf/briar/db/Database.java
index 425438081a..4f73cb9eb7 100644
--- a/components/net/sf/briar/db/Database.java
+++ b/components/net/sf/briar/db/Database.java
@@ -31,6 +31,7 @@ import net.sf.briar.api.transport.ConnectionWindow;
* <li> ratings
* <li> subscriptions
* <li> transports
+ * <li> windows
* </ul>
*/
interface Database<T> {
@@ -159,8 +160,10 @@ interface Database<T> {
/**
* Returns the connection reordering window for the given contact and
* transport.
+ * <p>
+ * Locking: contacts read, windows read.
*/
- ConnectionWindow getConnectionWindow(T txn, ContactId c, int transport)
+ ConnectionWindow getConnectionWindow(T txn, ContactId c, int transportId)
throws DbException;
/**
@@ -382,8 +385,10 @@ interface Database<T> {
/**
* Sets the connection reordering window for the given contact and
* transport.
+ * <p>
+ * Locking: contacts read, windows write.
*/
- void setConnectionWindow(T txn, ContactId c, int transport,
+ void setConnectionWindow(T txn, ContactId c, int transportId,
ConnectionWindow w) throws DbException;
/**
diff --git a/components/net/sf/briar/db/JdbcDatabase.java b/components/net/sf/briar/db/JdbcDatabase.java
index 5002ca4fd1..f2b314d68d 100644
--- a/components/net/sf/briar/db/JdbcDatabase.java
+++ b/components/net/sf/briar/db/JdbcDatabase.java
@@ -734,7 +734,7 @@ abstract class JdbcDatabase implements Database<Connection> {
}
public ConnectionWindow getConnectionWindow(Connection txn, ContactId c,
- int transport) throws DbException {
+ int transportId) throws DbException {
PreparedStatement ps = null;
ResultSet rs = null;
try {
@@ -742,7 +742,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " WHERE contactId = ? AND transportId = ?";
ps = txn.prepareStatement(sql);
ps.setInt(1, c.getInt());
- ps.setInt(2, transport);
+ ps.setInt(2, transportId);
rs = ps.executeQuery();
long centre = 0L;
int bitmap = 0;
@@ -1528,8 +1528,8 @@ abstract class JdbcDatabase implements Database<Connection> {
}
}
- public void setConnectionWindow(Connection txn, ContactId c, int transport,
- ConnectionWindow w) throws DbException {
+ public void setConnectionWindow(Connection txn, ContactId c,
+ int transportId, ConnectionWindow w) throws DbException {
PreparedStatement ps = null;
ResultSet rs = null;
try {
@@ -1537,7 +1537,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " WHERE contactId = ? AND transportId = ?";
ps = txn.prepareStatement(sql);
ps.setInt(1, c.getInt());
- ps.setInt(2, transport);
+ ps.setInt(2, transportId);
rs = ps.executeQuery();
if(rs.next()) {
if(rs.next()) throw new DbStateException();
@@ -1556,7 +1556,7 @@ abstract class JdbcDatabase implements Database<Connection> {
+ " VALUES(?, ?, ?, ?)";
ps = txn.prepareStatement(sql);
ps.setInt(1, c.getInt());
- ps.setInt(2, transport);
+ ps.setInt(2, transportId);
ps.setLong(3, w.getCentre());
ps.setInt(4, w.getBitmap());
int affected = ps.executeUpdate();
diff --git a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
index fbb44cd362..6a80b926c0 100644
--- a/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
+++ b/components/net/sf/briar/db/ReadWriteLockDatabaseComponent.java
@@ -32,6 +32,7 @@ import net.sf.briar.api.protocol.writers.OfferWriter;
import net.sf.briar.api.protocol.writers.RequestWriter;
import net.sf.briar.api.protocol.writers.SubscriptionWriter;
import net.sf.briar.api.protocol.writers.TransportWriter;
+import net.sf.briar.api.transport.ConnectionWindow;
import com.google.inject.Inject;
@@ -61,6 +62,8 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
new ReentrantReadWriteLock(true);
private final ReentrantReadWriteLock transportLock =
new ReentrantReadWriteLock(true);
+ private final ReentrantReadWriteLock windowLock =
+ new ReentrantReadWriteLock(true);
@Inject
ReadWriteLockDatabaseComponent(Database<Txn> db, DatabaseCleaner cleaner) {
@@ -488,6 +491,31 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
}
}
+ public ConnectionWindow getConnectionWindow(ContactId c, int transportId)
+ throws DbException {
+ contactLock.readLock().lock();
+ try {
+ if(!containsContact(c)) throw new NoSuchContactException();
+ windowLock.readLock().lock();
+ try {
+ Txn txn = db.startTransaction();
+ try {
+ ConnectionWindow w =
+ db.getConnectionWindow(txn, c, transportId);
+ db.commitTransaction(txn);
+ return w;
+ } catch(DbException e) {
+ db.abortTransaction(txn);
+ throw e;
+ }
+ } finally {
+ windowLock.readLock().unlock();
+ }
+ } finally {
+ contactLock.readLock().unlock();
+ }
+ }
+
public Collection<ContactId> getContacts() throws DbException {
contactLock.readLock().lock();
try {
@@ -869,6 +897,28 @@ class ReadWriteLockDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
}
}
+ public void setConnectionWindow(ContactId c, int transportId,
+ ConnectionWindow w) throws DbException {
+ contactLock.readLock().lock();
+ try {
+ if(!containsContact(c)) throw new NoSuchContactException();
+ windowLock.writeLock().lock();
+ try {
+ Txn txn = db.startTransaction();
+ try {
+ db.setConnectionWindow(txn, c, transportId, w);
+ db.commitTransaction(txn);
+ } catch(DbException e) {
+ db.abortTransaction(txn);
+ }
+ } finally {
+ windowLock.writeLock().unlock();
+ }
+ } finally {
+ contactLock.readLock().unlock();
+ }
+ }
+
public void setRating(AuthorId a, Rating r) throws DbException {
messageLock.writeLock().lock();
try {
diff --git a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
index d8680331bd..0bfc7db141 100644
--- a/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
+++ b/components/net/sf/briar/db/SynchronizedDatabaseComponent.java
@@ -31,6 +31,7 @@ import net.sf.briar.api.protocol.writers.OfferWriter;
import net.sf.briar.api.protocol.writers.RequestWriter;
import net.sf.briar.api.protocol.writers.SubscriptionWriter;
import net.sf.briar.api.protocol.writers.TransportWriter;
+import net.sf.briar.api.transport.ConnectionWindow;
import com.google.inject.Inject;
@@ -54,6 +55,7 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
private final Object ratingLock = new Object();
private final Object subscriptionLock = new Object();
private final Object transportLock = new Object();
+ private final Object windowLock = new Object();
@Inject
SynchronizedDatabaseComponent(Database<Txn> db, DatabaseCleaner cleaner) {
@@ -373,6 +375,25 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
}
}
+ public ConnectionWindow getConnectionWindow(ContactId c, int transportId)
+ throws DbException {
+ synchronized(contactLock) {
+ if(!containsContact(c)) throw new NoSuchContactException();
+ synchronized(windowLock) {
+ Txn txn = db.startTransaction();
+ try {
+ ConnectionWindow w =
+ db.getConnectionWindow(txn, c, transportId);
+ db.commitTransaction(txn);
+ return w;
+ } catch(DbException e) {
+ db.abortTransaction(txn);
+ throw e;
+ }
+ }
+ }
+ }
+
public Collection<ContactId> getContacts() throws DbException {
synchronized(contactLock) {
Txn txn = db.startTransaction();
@@ -659,6 +680,22 @@ class SynchronizedDatabaseComponent<Txn> extends DatabaseComponentImpl<Txn> {
}
}
+ public void setConnectionWindow(ContactId c, int transportId,
+ ConnectionWindow w) throws DbException {
+ synchronized(contactLock) {
+ if(!containsContact(c)) throw new NoSuchContactException();
+ synchronized(windowLock) {
+ Txn txn = db.startTransaction();
+ try {
+ db.setConnectionWindow(txn, c, transportId, w);
+ db.commitTransaction(txn);
+ } catch(DbException e) {
+ db.abortTransaction(txn);
+ }
+ }
+ }
+ }
+
public void setRating(AuthorId a, Rating r) throws DbException {
synchronized(messageLock) {
synchronized(ratingLock) {
diff --git a/components/net/sf/briar/invitation/InvitationWorker.java b/components/net/sf/briar/invitation/InvitationWorker.java
index dc6d5615d4..33df74c059 100644
--- a/components/net/sf/briar/invitation/InvitationWorker.java
+++ b/components/net/sf/briar/invitation/InvitationWorker.java
@@ -20,16 +20,15 @@ class InvitationWorker implements Runnable {
private final InvitationCallback callback;
private final InvitationParameters parameters;
- private final DatabaseComponent databaseComponent;
+ private final DatabaseComponent db;
private final WriterFactory writerFactory;
InvitationWorker(final InvitationCallback callback,
- InvitationParameters parameters,
- DatabaseComponent databaseComponent,
+ InvitationParameters parameters, DatabaseComponent db,
WriterFactory writerFactory) {
this.callback = callback;
this.parameters = parameters;
- this.databaseComponent = databaseComponent;
+ this.db = db;
this.writerFactory = writerFactory;
}
@@ -73,7 +72,7 @@ class InvitationWorker implements Runnable {
// FIXME: Create a real invitation
Map<String, Map<String, String>> transports;
try {
- transports = databaseComponent.getTransports();
+ transports = db.getTransports();
} catch(DbException e) {
throw new IOException(e.getMessage());
}
diff --git a/test/net/sf/briar/db/DatabaseComponentTest.java b/test/net/sf/briar/db/DatabaseComponentTest.java
index f12acc7666..4a8bd4ee46 100644
--- a/test/net/sf/briar/db/DatabaseComponentTest.java
+++ b/test/net/sf/briar/db/DatabaseComponentTest.java
@@ -11,8 +11,8 @@ import net.sf.briar.TestUtils;
import net.sf.briar.api.ContactId;
import net.sf.briar.api.Rating;
import net.sf.briar.api.db.DatabaseComponent;
-import net.sf.briar.api.db.DbException;
import net.sf.briar.api.db.DatabaseListener;
+import net.sf.briar.api.db.DbException;
import net.sf.briar.api.db.NoSuchContactException;
import net.sf.briar.api.db.Status;
import net.sf.briar.api.protocol.Ack;
@@ -32,6 +32,7 @@ import net.sf.briar.api.protocol.writers.OfferWriter;
import net.sf.briar.api.protocol.writers.RequestWriter;
import net.sf.briar.api.protocol.writers.SubscriptionWriter;
import net.sf.briar.api.protocol.writers.TransportWriter;
+import net.sf.briar.api.transport.ConnectionWindow;
import org.jmock.Expectations;
import org.jmock.Mockery;
@@ -81,6 +82,8 @@ public abstract class DatabaseComponentTest extends TestCase {
@SuppressWarnings("unchecked")
final Database<Object> database = context.mock(Database.class);
final DatabaseCleaner cleaner = context.mock(DatabaseCleaner.class);
+ final ConnectionWindow connectionWindow =
+ context.mock(ConnectionWindow.class);
final Group group = context.mock(Group.class);
final DatabaseListener listener = context.mock(DatabaseListener.class);
context.checking(new Expectations() {{
@@ -99,6 +102,11 @@ public abstract class DatabaseComponentTest extends TestCase {
// getContacts()
oneOf(database).getContacts(txn);
will(returnValue(Collections.singletonList(contactId)));
+ // getConnectionWindow(contactId, 123)
+ oneOf(database).containsContact(txn, contactId);
+ will(returnValue(true));
+ oneOf(database).getConnectionWindow(txn, contactId, 123);
+ will(returnValue(connectionWindow));
// getTransports(contactId)
oneOf(database).containsContact(txn, contactId);
will(returnValue(true));
@@ -129,6 +137,11 @@ public abstract class DatabaseComponentTest extends TestCase {
// unsubscribe(groupId) again
oneOf(database).containsSubscription(txn, groupId);
will(returnValue(false));
+ // setConnectionWindow(contactId, 123, connectionWindow)
+ oneOf(database).containsContact(txn, contactId);
+ will(returnValue(true));
+ oneOf(database).setConnectionWindow(txn, contactId, 123,
+ connectionWindow);
// removeContact(contactId)
oneOf(database).removeContact(txn, contactId);
// close()
@@ -142,12 +155,14 @@ public abstract class DatabaseComponentTest extends TestCase {
assertEquals(Rating.UNRATED, db.getRating(authorId));
assertEquals(contactId, db.addContact(transports));
assertEquals(Collections.singletonList(contactId), db.getContacts());
+ assertEquals(connectionWindow, db.getConnectionWindow(contactId, 123));
assertEquals(transports, db.getTransports(contactId));
db.subscribe(group);
db.subscribe(group); // Again - check listeners aren't called
assertEquals(Collections.singletonList(groupId), db.getSubscriptions());
db.unsubscribe(groupId);
db.unsubscribe(groupId); // Again - check listeners aren't called
+ db.setConnectionWindow(contactId, 123, connectionWindow);
db.removeContact(contactId);
db.removeListener(listener);
db.close();
@@ -472,11 +487,11 @@ public abstract class DatabaseComponentTest extends TestCase {
context.checking(new Expectations() {{
// Check whether the contact is still in the DB (which it's not)
// once for each method
- exactly(14).of(database).startTransaction();
+ exactly(16).of(database).startTransaction();
will(returnValue(txn));
- exactly(14).of(database).containsContact(txn, contactId);
+ exactly(16).of(database).containsContact(txn, contactId);
will(returnValue(false));
- exactly(14).of(database).commitTransaction(txn);
+ exactly(16).of(database).commitTransaction(txn);
}});
DatabaseComponent db = createDatabaseComponent(database, cleaner);
@@ -516,6 +531,11 @@ public abstract class DatabaseComponentTest extends TestCase {
fail();
} catch(NoSuchContactException expected) {}
+ try {
+ db.getConnectionWindow(contactId, 123);
+ fail();
+ } catch(NoSuchContactException expected) {}
+
try {
db.getTransports(contactId);
fail();
@@ -551,6 +571,11 @@ public abstract class DatabaseComponentTest extends TestCase {
fail();
} catch(NoSuchContactException expected) {}
+ try {
+ db.setConnectionWindow(contactId, 123, null);
+ fail();
+ } catch(NoSuchContactException expected) {}
+
context.assertIsSatisfied();
}
diff --git a/test/net/sf/briar/transport/ConnectionWindowImplTest.java b/test/net/sf/briar/transport/ConnectionWindowImplTest.java
index c95dc19edd..54595b2342 100644
--- a/test/net/sf/briar/transport/ConnectionWindowImplTest.java
+++ b/test/net/sf/briar/transport/ConnectionWindowImplTest.java
@@ -8,6 +8,8 @@ import org.junit.Test;
public class ConnectionWindowImplTest extends TestCase {
+ private static final long MAX_32_BIT_UNSIGNED = 4294967295L; // 2^32 - 1
+
@Test
public void testWindowSliding() {
ConnectionWindowImpl w = new ConnectionWindowImpl(0L, 0);
@@ -40,6 +42,13 @@ public class ConnectionWindowImplTest extends TestCase {
w.setSeen(48);
fail();
} catch(IllegalArgumentException expected) {}
+ w = new ConnectionWindowImpl(MAX_32_BIT_UNSIGNED - 1, 0);
+ // Values greater than 2^31 - 1 should never be allowed
+ w.setSeen(MAX_32_BIT_UNSIGNED);
+ try {
+ w.setSeen(MAX_32_BIT_UNSIGNED + 1);
+ fail();
+ } catch(IllegalArgumentException expected) {}
}
@Test
--
GitLab