/hg/icedtea-web: Modified CacheEntry and CacheLRUWrapper unit te...
jkang at icedtea.classpath.org
jkang at icedtea.classpath.org
Tue Oct 7 19:15:16 UTC 2014
changeset 6bbd07a0b15a in /hg/icedtea-web
details: http://icedtea.classpath.org/hg/icedtea-web?cmd=changeset;node=6bbd07a0b15a
author: Jie Kang <jkang at redhat.com>
date: Tue Oct 07 15:13:17 2014 -0400
Modified CacheEntry and CacheLRUWrapper unit tests to prevent blocking.
2014-10-07 Jie Kang <jkang at redhat.com>
Modified unit tests for CacheEntry and CacheLRUWrapper to prevent blocking.
* netx/net/sourceforge/jnlp/cache/CacheEntry.java: comments for unlocking
* netx/net/sourceforge/jnlp/util/PropertiesFile.java: same
* netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java: same
* tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java: fixed
multi-threaded tests to prevent blocking, and added timeout to threaded
tests to prevent tests from blocking test runs
* tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java: same
* tests/netx/unit/net/sourceforge/jnlp/cache/CacheTestUtils.java: utilities
for cache tests
diffstat:
ChangeLog | 13 +
netx/net/sourceforge/jnlp/cache/CacheEntry.java | 2 +-
netx/net/sourceforge/jnlp/util/PropertiesFile.java | 4 +
netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java | 2 +-
tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java | 139 +++++----
tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java | 122 ++++----
tests/test-extensions/net/sourceforge/jnlp/util/CacheTestUtils.java | 49 +++
7 files changed, 202 insertions(+), 129 deletions(-)
diffs (truncated from 585 to 500 lines):
diff -r 6f4c1d501560 -r 6bbd07a0b15a ChangeLog
--- a/ChangeLog Tue Oct 07 14:41:11 2014 -0400
+++ b/ChangeLog Tue Oct 07 15:13:17 2014 -0400
@@ -1,3 +1,16 @@
+2014-10-07 Jie Kang <jkang at redhat.com>
+
+ Modified unit tests for CacheEntry and CacheLRUWrapper to prevent blocking.
+ * netx/net/sourceforge/jnlp/cache/CacheEntry.java: comments for unlocking
+ * netx/net/sourceforge/jnlp/util/PropertiesFile.java: same
+ * netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java: same
+ * tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java: fixed
+ multi-threaded tests to prevent blocking, and added timeout to threaded
+ tests to prevent tests from blocking test runs
+ * tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java: same
+ * tests/netx/unit/net/sourceforge/jnlp/cache/CacheTestUtils.java: utilities
+ for cache tests
+
2014-10-07 Jie Kang <jkang at redhat.com>
Changed ResourceTracker to use cached thread pool as opposed to manual
diff -r 6f4c1d501560 -r 6bbd07a0b15a netx/net/sourceforge/jnlp/cache/CacheEntry.java
--- a/netx/net/sourceforge/jnlp/cache/CacheEntry.java Tue Oct 07 14:41:11 2014 -0400
+++ b/netx/net/sourceforge/jnlp/cache/CacheEntry.java Tue Oct 07 15:13:17 2014 -0400
@@ -239,7 +239,7 @@
}
/**
- * Unlock cache item.
+ * Unlock cache item. Does not do anything if not holding the lock.
*/
protected void unlock() {
properties.unlock();
diff -r 6f4c1d501560 -r 6bbd07a0b15a netx/net/sourceforge/jnlp/util/PropertiesFile.java
--- a/netx/net/sourceforge/jnlp/util/PropertiesFile.java Tue Oct 07 14:41:11 2014 -0400
+++ b/netx/net/sourceforge/jnlp/util/PropertiesFile.java Tue Oct 07 15:13:17 2014 -0400
@@ -192,6 +192,10 @@
return false;
}
+ /**
+ * Unlocks the file. Does not do anything if not holding the lock.
+ */
+
public void unlock() {
try {
lockedFile.unlock();
diff -r 6f4c1d501560 -r 6bbd07a0b15a netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java
--- a/netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java Tue Oct 07 14:41:11 2014 -0400
+++ b/netx/net/sourceforge/jnlp/util/lockingfile/LockedFile.java Tue Oct 07 15:13:17 2014 -0400
@@ -150,7 +150,7 @@
}
/**
- * Unlock access to the file. Lock is reentrant.
+ * Unlock access to the file. Lock is reentrant. Does not do anything if not holding the lock.
*/
public void unlock() throws IOException {
if (JNLPRuntime.isWindows() || !this.threadLock.isHeldByCurrentThread()) {
diff -r 6f4c1d501560 -r 6bbd07a0b15a tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java Tue Oct 07 14:41:11 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheEntryTest.java Tue Oct 07 15:13:17 2014 -0400
@@ -40,6 +40,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import java.io.ByteArrayOutputStream;
import java.io.File;
@@ -48,13 +49,13 @@
import java.net.MalformedURLException;
import java.net.URL;
import java.nio.file.Files;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.util.concurrent.CountDownLatch;
import org.junit.Before;
import org.junit.Test;
import net.sourceforge.jnlp.Version;
+import net.sourceforge.jnlp.util.CacheTestUtils;
import net.sourceforge.jnlp.util.PropertiesFile;
public class CacheEntryTest {
@@ -85,10 +86,6 @@
private ByteArrayOutputStream baos;
private PrintStream out;
- private ExecutorService executorService;
- Object listener = new Object();
-
- int num = 0;
@Before
public void setUp() throws MalformedURLException {
@@ -96,7 +93,6 @@
version = new Version("1.0");
baos = new ByteArrayOutputStream();
out = new PrintStream(baos);
- executorService = Executors.newSingleThreadExecutor();
}
@Test
@@ -222,7 +218,7 @@
return cachedFile;
}
- @Test
+ @Test(timeout = 2000l)
public void testLock() throws IOException {
TestCacheEntry entry = new TestCacheEntry(url, version, null);
try {
@@ -233,7 +229,7 @@
}
}
- @Test
+ @Test(timeout = 2000l)
public void testUnlock() throws IOException {
TestCacheEntry entry = new TestCacheEntry(url, version, null);
try {
@@ -244,7 +240,7 @@
assertTrue(!entry.isHeldByCurrentThread());
}
- @Test
+ @Test(timeout = 2000l)
public void testStoreFailsWithoutLock() throws IOException {
TestCacheEntry entry = new TestCacheEntry(url, version, null);
long num = 10;
@@ -252,7 +248,7 @@
assertTrue(!entry.store());
}
- @Test
+ @Test(timeout = 2000l)
public void testStoreWorksWithLocK() throws IOException {
TestCacheEntry entry = new TestCacheEntry(url, version, null);
long num = 10;
@@ -265,99 +261,120 @@
}
}
- @Test
+ @Test(timeout = 2000l)
public void testMultithreadLockPreventsWrite() throws IOException, InterruptedException {
+ int numThreads = 100;
+ CountDownLatch doneSignal = new CountDownLatch(numThreads);
+ CountDownLatch writersDoneSignal = new CountDownLatch(numThreads);
+
TestCacheEntry entry = new TestCacheEntry(url, version, null);
- Thread a = new Thread(new WriteWorker(10, entry));
- a.start();
+ Thread[] list = new Thread[numThreads];
- Thread b = new Thread(new WriteWorker(5, entry));
- b.start();
+ for (int i=0; i<numThreads; i++) {
+ list[i] = new Thread(new WriteWorker(i, entry, doneSignal, writersDoneSignal));
+ }
- Thread.sleep(2000l);
+ for (int i=0; i<numThreads; i++) {
+ list[i].start();
+ }
- synchronized (listener) {
- num = 1;
- listener.notifyAll();
+ //Wait for all children to finish
+ for (int i = 0; i < numThreads; i++) {
+ list[i].join();
}
String out = baos.toString();
- assertTrue(out.contains(":10:true") && out.contains(":5:false"));
+ assertTrue(CacheTestUtils.stringContainsOnlySingleInstance(out, "true") && out.contains("false"));
}
- @Test
+
+ @Test(timeout = 2000l)
public void testMultithreadLockAllowsRead() throws IOException, InterruptedException {
+ int numThreads = 2;
+ int numWriterThreads = 1;
+
+ CountDownLatch doneSignal = new CountDownLatch(numThreads);
+ CountDownLatch writersDoneSignal = new CountDownLatch(numWriterThreads);
+
TestCacheEntry entry = new TestCacheEntry(url, version, null);
- Thread a = new Thread(new WriteWorker(10, entry));
- a.start();
- Thread.sleep(2000l);
+ Thread writerThread = new Thread(new WriteWorker(10, entry, doneSignal, writersDoneSignal));
+ writerThread.start();
- Thread b = new Thread(new ReadWorker(entry));
- b.start();
+ Thread readerThread = new Thread(new ReadWorker(entry, writersDoneSignal, doneSignal));
+ readerThread.start();
- Thread.sleep(1000l);
-
- synchronized (listener) {
- num = 1;
- listener.notifyAll();
- }
+ writerThread.join();
+ readerThread.join();
String out = baos.toString();
assertTrue(out.contains(":10:true") && out.contains(":read:10"));
}
private class WriteWorker implements Runnable {
- long input;
- TestCacheEntry entry;
+ private final long input;
+ private final TestCacheEntry entry;
+ private final CountDownLatch doneSignal;
+ private final CountDownLatch writersDoneSignal;
- public WriteWorker(long input, TestCacheEntry entry) {
+ public WriteWorker(long input, TestCacheEntry entry, CountDownLatch doneSignal, CountDownLatch writersDoneSignal) {
this.input = input;
this.entry = entry;
+ this.doneSignal = doneSignal;
+ this.writersDoneSignal = writersDoneSignal;
}
@Override
public void run() {
-
try {
- boolean result = entry.tryLock();
+ entry.tryLock();
entry.setLastModified(input);
- result = entry.store();
- executorService.execute(new WriteRunnable(":" + input + ":" + result));
- while (num == 0) {
- synchronized (listener) {
- listener.wait();
- }
+ boolean result = entry.store();
+ synchronized (out) {
+ out.println(":" + input + ":" + result);
+ out.flush();
}
+ //Let parent know outputting is done
+ doneSignal.countDown();
+ writersDoneSignal.countDown();
+ //Wait until everyone is done before continuing to clw.unlock()
+ doneSignal.await();
} catch (Exception e) {
e.printStackTrace();
+ fail();
} finally {
entry.unlock();
}
}
}
- private class WriteRunnable implements Runnable {
- private String msg;
- public WriteRunnable(String msg) {
- this.msg = msg;
+ private class ReadWorker implements Runnable {
+ private final TestCacheEntry entry;
+
+ private final CountDownLatch writersDone;
+ private final CountDownLatch doneSignal;
+
+ public ReadWorker(TestCacheEntry entry, CountDownLatch writersDone, CountDownLatch doneSignal) {
+ this.entry = entry;
+ this.writersDone = writersDone;
+ this.doneSignal = doneSignal;
}
@Override
public void run() {
- out.println(msg);
- out.flush();
- }
- }
+ try {
+ writersDone.await();
- private class ReadWorker implements Runnable {
- private TestCacheEntry entry;
- public ReadWorker(TestCacheEntry entry) {
- this.entry = entry;
- }
- @Override
- public void run() {
- long lastModified = entry.getLastModified();
- executorService.execute(new WriteRunnable(":read:" + lastModified));
+ long lastModified = entry.getLastModified();
+ synchronized (out) {
+ out.println(":read:" + lastModified);
+ out.flush();
+ doneSignal.countDown();
+ doneSignal.await();
+ }
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ fail();
+ }
}
}
}
diff -r 6f4c1d501560 -r 6bbd07a0b15a tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java
--- a/tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java Tue Oct 07 14:41:11 2014 -0400
+++ b/tests/netx/unit/net/sourceforge/jnlp/cache/CacheLRUWrapperTest.java Tue Oct 07 15:13:17 2014 -0400
@@ -44,8 +44,7 @@
import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
+import java.util.concurrent.CountDownLatch;
import org.junit.AfterClass;
import org.junit.Before;
@@ -53,6 +52,7 @@
import org.junit.Test;
import net.sourceforge.jnlp.ServerAccess;
+import net.sourceforge.jnlp.util.CacheTestUtils;
import net.sourceforge.jnlp.util.PropertiesFile;
public class CacheLRUWrapperTest {
@@ -67,10 +67,6 @@
private ByteArrayOutputStream baos;
private PrintStream out;
- private ExecutorService executorService;
- Object listener = new Object();
-
- int num = 0;
@BeforeClass
static public void setupJNLPRuntimeConfig() {
@@ -91,7 +87,6 @@
public void setup() {
baos = new ByteArrayOutputStream();
out = new PrintStream(baos);
- executorService = Executors.newSingleThreadExecutor();
}
@Test
@@ -99,36 +94,35 @@
final File cacheIndexFile = new File(clw.cacheDir + File.separator + cacheIndexFileName);
cacheIndexFile.delete();
- try{
-
- int noLoops = 1000;
+ try {
+ int noLoops = 1000;
- long time[] = new long[noLoops];
+ long time[] = new long[noLoops];
- clw.lock();
- clearCacheIndexFile();
+ clw.lock();
+ clearCacheIndexFile();
- fillCacheIndexFile(noEntriesCacheFile);
- clw.store();
+ fillCacheIndexFile(noEntriesCacheFile);
+ clw.store();
- // FIXME: wait a second, because of file modification timestamp only provides accuracy on seconds.
- Thread.sleep(1000);
+ // FIXME: wait a second, because of file modification timestamp only provides accuracy on seconds.
+ Thread.sleep(1000);
- long sum = 0;
- for(int i=0; i < noLoops - 1; i++) {
- time[i]= System.nanoTime();
- clw.load();
- time[i+1]= System.nanoTime();
- if(i==0)
- continue;
- sum = sum + time[i] - time[i-1];
- }
-
- double avg = sum / time.length;
- ServerAccess.logErrorReprint("Average = " + avg + "ns");
+ long sum = 0;
+ for(int i=0; i < noLoops - 1; i++) {
+ time[i]= System.nanoTime();
+ clw.load();
+ time[i+1]= System.nanoTime();
+ if(i==0)
+ continue;
+ sum = sum + time[i] - time[i-1];
+ }
- // wait more than 100 microseconds for noLoops = 1000 and noEntries=1000 is bad
- assertTrue("load() must not take longer than 100 µs, but took in avg " + avg/1000 + "µs", avg < 100 * 1000);
+ double avg = sum / time.length;
+ ServerAccess.logErrorReprint("Average = " + avg + "ns");
+
+ // wait more than 100 microseconds for noLoops = 1000 and noEntries=1000 is bad
+ assertTrue("load() must not take longer than 100 µs, but took in avg " + avg/1000 + "µs", avg < 100 * 1000);
} finally {
clw.unlock();
cacheIndexFile.delete();
@@ -216,7 +210,7 @@
assertFalse(clw.containsKey(key) && clw.containsValue(value));
}
- @Test
+ @Test(timeout = 2000l)
public void testLock() throws IOException {
try {
clw.lock();
@@ -226,7 +220,7 @@
}
}
- @Test
+ @Test(timeout = 2000l)
public void testUnlock() throws IOException {
try {
clw.lock();
@@ -236,12 +230,12 @@
assertTrue(!clw.cacheOrder.isHeldByCurrentThread());
}
- @Test
+ @Test(timeout = 2000l)
public void testStoreFailsWithoutLock() throws IOException {
assertTrue(!clw.store());
}
- @Test
+ @Test(timeout = 2000l)
public void testStoreWorksWithLocK() throws IOException {
try {
clw.lock();
@@ -251,41 +245,51 @@
}
}
- @Test
+ @Test(timeout = 2000l)
public void testMultithreadLockPreventsStore() throws IOException, InterruptedException {
- Thread a = new Thread(new StoreWorker());
- a.start();
+ int numThreads = 100;
+ CountDownLatch doneSignal = new CountDownLatch(numThreads);
- Thread b = new Thread(new StoreWorker());
- b.start();
+ Thread[] list = new Thread[numThreads];
- Thread.sleep(2000l);
+ for (int i = 0; i < numThreads; i++) {
+ list[i] = new Thread(new StoreWorker(doneSignal));
+ }
- synchronized (listener) {
- num = 1;
- listener.notifyAll();
+ for (int i = 0; i < numThreads; i++) {
+ list[i].start();
+ }
+
+ //Wait for all children to finish
+ for (int i = 0; i < numThreads; i++) {
+ list[i].join();
}
String out = baos.toString();
- System.out.println();
- assertTrue(out.contains("true") && out.contains("false"));
+
+ assertTrue(CacheTestUtils.stringContainsOnlySingleInstance(out, "true") && out.contains("false"));
}
private class StoreWorker implements Runnable {
- public StoreWorker() {
+ private final CountDownLatch doneSignal;
+
+ public StoreWorker(CountDownLatch doneSignal) {
+ this.doneSignal = doneSignal;
}
@Override
public void run() {
try {
clw.cacheOrder.tryLock();
boolean result = clw.store();
- executorService.execute(new WriteRunnable(String.valueOf(result)));
- while (num == 0) {
- synchronized (listener) {
More information about the distro-pkg-dev
mailing list