diff --git a/CHANGELOG.md b/CHANGELOG.md index b842574af4c4b190614c0c75e5bf061b1d47171e..305dd69b00b84c30b5caa91dec9f97b6beaa4b92 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +* #122 - Dead-lock due to error in the DatabaseAccessManager. * #120 - Editing entities that were created with a no longer existing user leads to a server error. * #31 - Queries with keywords in the path (e.g. `... STORED AT 0in.txt`) diff --git a/src/main/java/org/caosdb/server/database/DatabaseAccessManager.java b/src/main/java/org/caosdb/server/database/DatabaseAccessManager.java index 196a9fb2b4418d81ced4b174482f8f6a5dce8ce8..f12c78e31bf28799b8c8ead1c9ff311a65bb01ef 100644 --- a/src/main/java/org/caosdb/server/database/DatabaseAccessManager.java +++ b/src/main/java/org/caosdb/server/database/DatabaseAccessManager.java @@ -26,6 +26,7 @@ package org.caosdb.server.database; import java.util.concurrent.Semaphore; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; import org.caosdb.server.database.access.Access; import org.caosdb.server.database.access.AccessControlAccess; @@ -56,7 +57,7 @@ import org.caosdb.server.utils.Releasable; class ReadAccessSemaphore extends Semaphore implements Releasable { private static final long serialVersionUID = 4384921156838881337L; - private int acquired = 0; // how many threads have read access + private AtomicInteger acquired = new AtomicInteger(0); // how many threads have read access Semaphore writersBlock = new Semaphore(1, true); // This semaphore is blocked as long as there are any // unreleased read permits. @@ -73,10 +74,9 @@ class ReadAccessSemaphore extends Semaphore implements Releasable { @Override public void acquire() throws InterruptedException { super.acquire(); // Protect the next few lines - if (this.acquired == 0) { + if (this.acquired.getAndIncrement() == 0) { this.writersBlock.acquire(); } - this.acquired++; super.release(); } @@ -87,9 +87,7 @@ class ReadAccessSemaphore extends Semaphore implements Releasable { */ @Override public void release() { - this.acquired--; - if (this.acquired <= 0) { // Last permit: release - this.acquired = 0; + if (this.acquired.decrementAndGet() == 0) { // Last permit: release if (this.writersBlock.availablePermits() <= 0) { this.writersBlock.release(); } diff --git a/src/test/java/org/caosdb/server/database/DatabaseAccessManagerTest.java b/src/test/java/org/caosdb/server/database/DatabaseAccessManagerTest.java index 8e08131f3b18d9e4a495301ee570683facb4d9c5..0ba085b1fd050578b50e443613f82167890ea61f 100644 --- a/src/test/java/org/caosdb/server/database/DatabaseAccessManagerTest.java +++ b/src/test/java/org/caosdb/server/database/DatabaseAccessManagerTest.java @@ -22,10 +22,91 @@ */ package org.caosdb.server.database; +import static org.junit.Assert.assertFalse; + +import java.util.LinkedList; +import java.util.List; import org.junit.Assert; import org.junit.Test; public class DatabaseAccessManagerTest { + Thread createReadThread(long wait, String name, ReadAccessSemaphore readAccess) { + return new Thread( + new Runnable() { + + @Override + public void run() { + try { + readAccess.acquire(); + Thread.sleep(wait); + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + readAccess.release(); + } + } + }, + name); + } + + Thread createWriteThread(long wait, String name, WriteAccessLock writeAccess) { + return new Thread( + new Runnable() { + + @Override + public void run() { + try { + writeAccess.reserve(); + Thread.sleep(wait); + writeAccess.lockInterruptibly(); + Thread.sleep(wait); + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + writeAccess.release(); + } + } + }, + name); + } + + @Test + public void testDeadLock() throws InterruptedException { + final ReadAccessSemaphore readAccess = new ReadAccessSemaphore(); + final WriteAccessLock writeAccess = new WriteAccessLock(readAccess); + List<Thread> ts = new LinkedList<>(); + for (int i = 0; i < 1000; i++) { + Thread t1 = createReadThread(1, "Ra" + i, readAccess); + Thread t2 = createReadThread(2, "Rb" + i, readAccess); + Thread t3 = createReadThread(3, "Rc" + i, readAccess); + Thread t5 = createReadThread(5, "Rd" + i, readAccess); + Thread t7 = createReadThread(7, "Re" + i, readAccess); + Thread t11 = createReadThread(11, "Rf" + i, readAccess); + Thread w5 = createWriteThread(2, "W" + i, writeAccess); + + t1.start(); + t2.start(); + w5.start(); + t3.start(); + t5.start(); + t7.start(); + t11.start(); + + ts.add(t1); + ts.add(t2); + ts.add(t3); + ts.add(t5); + ts.add(t7); + ts.add(t11); + ts.add(w5); + } + + for (Thread t : ts) { + t.join(10000); + assertFalse(t.isAlive()); + } + } + public static final ReadAccessSemaphore readAccess = new ReadAccessSemaphore(); public static final WriteAccessLock writeAccess = new WriteAccessLock(readAccess); diff --git a/src/test/java/org/caosdb/server/query/QueryTest.java b/src/test/java/org/caosdb/server/query/QueryTest.java index da652cb3e9bed9b2412dae116bb104c034a01cfe..26e43ec6c65414808c72b9e04b95c1d67011fc9a 100644 --- a/src/test/java/org/caosdb/server/query/QueryTest.java +++ b/src/test/java/org/caosdb/server/query/QueryTest.java @@ -70,8 +70,8 @@ public class QueryTest { /** * Assure that {@link WriteTransaction#commit()} calls {@link Query#clearCache()}. * - * Since currently the cache shall be cleared whenever there is a commit. - * */ + * <p>Since currently the cache shall be cleared whenever there is a commit. + */ @Test public void testEtagChangesAfterWrite() { String old = Query.getETag();