diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java index d00159903596c83da6d3750dc115571dcab6215e..42470404685e9bde347c0c3e74e58cc780b0550e 100644 --- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java +++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java @@ -59,7 +59,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.logging.Logger; @@ -80,7 +79,8 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { private final EventBus eventBus; private final ShutdownManager shutdown; private final AtomicBoolean closed = new AtomicBoolean(false); - private final ReadWriteLock lock = new ReentrantReadWriteLock(true); + private final ReentrantReadWriteLock lock = + new ReentrantReadWriteLock(true); private volatile int shutdownHandle = -1; @@ -119,6 +119,9 @@ class DatabaseComponentImpl<T> implements DatabaseComponent { } public Transaction startTransaction(boolean readOnly) throws DbException { + // Don't allow reentrant locking + if (lock.getReadHoldCount() > 0) throw new IllegalStateException(); + if (lock.getWriteHoldCount() > 0) throw new IllegalStateException(); if (readOnly) lock.readLock().lock(); else lock.writeLock().lock(); try { diff --git a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java index 02531d08d1165f34c1aaeea85ece32a9fd43cbeb..eed704adde2acd75842164557f0311ad359b6514 100644 --- a/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java +++ b/briar-tests/src/org/briarproject/db/DatabaseComponentImplTest.java @@ -65,6 +65,7 @@ import static org.briarproject.api.transport.TransportConstants.REORDERING_WINDO import static org.briarproject.db.DatabaseConstants.MAX_OFFERED_MESSAGES; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; public class DatabaseComponentImplTest extends BriarTestCase { @@ -1447,4 +1448,53 @@ public class DatabaseComponentImplTest extends BriarTestCase { context.assertIsSatisfied(); } + + @Test(expected = IllegalStateException.class) + public void testCannotStartReadTransactionDuringReadTransaction() + throws Exception { + testCannotStartTransactionDuringTransaction(true, true); + } + + @Test(expected = IllegalStateException.class) + public void testCannotStartWriteTransactionDuringReadTransaction() + throws Exception { + testCannotStartTransactionDuringTransaction(true, false); + } + + @Test(expected = IllegalStateException.class) + public void testCannotStartReadTransactionDuringWriteTransaction() + throws Exception { + testCannotStartTransactionDuringTransaction(false, true); + } + + @Test(expected = IllegalStateException.class) + public void testCannotStartWriteTransactionDuringWriteTransaction() + throws Exception { + testCannotStartTransactionDuringTransaction(false, false); + } + + private void testCannotStartTransactionDuringTransaction( + boolean firstTxnReadOnly, boolean secondTxnReadOnly) + throws Exception { + Mockery context = new Mockery(); + @SuppressWarnings("unchecked") + final Database<Object> database = context.mock(Database.class); + final ShutdownManager shutdown = context.mock(ShutdownManager.class); + final EventBus eventBus = context.mock(EventBus.class); + + context.checking(new Expectations() {{ + oneOf(database).startTransaction(); + will(returnValue(new Object())); + }}); + + DatabaseComponent db = createDatabaseComponent(database, eventBus, + shutdown); + + assertNotNull(db.startTransaction(firstTxnReadOnly)); + try { + db.startTransaction(secondTxnReadOnly); + } finally { + context.assertIsSatisfied(); + } + } }