Updated Branches: refs/heads/wicket-1.5.x 44c078336 -> c6f7651cc
WICKET-4572 DiskDataStore returns the wrong page when the page disk space is full Synchronize the public methods in PageWindowManager. Each Http Session has its own PageWindowManager instance. A blocking may occur when two or more requests in the same session touch different page instances or when the Asynchronous Queue used for writing pages' bytes is full. Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/c6f7651c Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/c6f7651c Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/c6f7651c Branch: refs/heads/wicket-1.5.x Commit: c6f7651cc0fb1e3d9bea803b80404a8421c8df70 Parents: 44c0783 Author: Martin Tzvetanov Grigorov <[email protected]> Authored: Wed May 30 16:06:36 2012 +0300 Committer: Martin Tzvetanov Grigorov <[email protected]> Committed: Wed May 30 16:06:36 2012 +0300 ---------------------------------------------------------------------- .../apache/wicket/pageStore/PageWindowManager.java | 8 +- .../persistent/disk/PageWindowManagerTest.java | 117 +++++++++++++++ .../pageStore/AsynchronousDataStoreTest.java | 6 +- .../apache/wicket/pageStore/DiskDataStoreTest.java | 1 - .../wicket/versioning/InMemoryPageStore.java | 5 +- .../java/org/apache/wicket/util/file/Files.java | 1 + 6 files changed, 129 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-core/src/main/java/org/apache/wicket/pageStore/PageWindowManager.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/PageWindowManager.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/PageWindowManager.java index 03887bb..f194710 100644 --- a/wicket-core/src/main/java/org/apache/wicket/pageStore/PageWindowManager.java +++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/PageWindowManager.java @@ -371,7 +371,7 @@ public class PageWindowManager implements Serializable * @param size * @return page window */ - public PageWindow createPageWindow(int pageId, int size) + public synchronized PageWindow createPageWindow(int pageId, int size) { int index = getWindowIndex(pageId); @@ -403,7 +403,7 @@ public class PageWindowManager implements Serializable * @param pageId * @return page window or null */ - public PageWindow getPageWindow(int pageId) + public synchronized PageWindow getPageWindow(int pageId) { int index = getWindowIndex(pageId); if (index != -1) @@ -418,7 +418,7 @@ public class PageWindowManager implements Serializable * * @param pageId */ - public void removePage(int pageId) + public synchronized void removePage(int pageId) { int index = getWindowIndex(pageId); if (index != -1) @@ -499,7 +499,7 @@ public class PageWindowManager implements Serializable * * @return total size */ - public int getTotalSize() + public synchronized int getTotalSize() { return totalSize; } http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-core/src/test/java/org/apache/wicket/page/persistent/disk/PageWindowManagerTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/page/persistent/disk/PageWindowManagerTest.java b/wicket-core/src/test/java/org/apache/wicket/page/persistent/disk/PageWindowManagerTest.java index 44b5d71..7ede689 100644 --- a/wicket-core/src/test/java/org/apache/wicket/page/persistent/disk/PageWindowManagerTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/page/persistent/disk/PageWindowManagerTest.java @@ -16,6 +16,11 @@ */ package org.apache.wicket.page.persistent.disk; +import java.security.SecureRandom; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + import org.apache.wicket.pageStore.PageWindowManager; import org.apache.wicket.pageStore.PageWindowManager.PageWindow; import org.junit.Assert; @@ -180,4 +185,116 @@ public class PageWindowManagerTest extends Assert assertTrue(window.getPageId() == pageId && window.getFilePartOffset() == filePartOffset && window.getFilePartSize() == filePartSize); } + + /** how many operations to execute */ + private static final int EXECUTIONS = 10000; + + /** used to wait the executions */ + private static final CountDownLatch LATCH = new CountDownLatch(EXECUTIONS); + + private final PageWindowManager pageWindowManager = new PageWindowManager(1000L); + + /** the execution types */ + private final Runnable[] TASKS = new Runnable[] + { + new CreatePageWindowTask(pageWindowManager), + new GetPageWindowTask(pageWindowManager), + new RemovePageInSessionTask(pageWindowManager) + }; + + private static final SecureRandom RND = new SecureRandom(); + + /** + * Executes random mutator and accessor operations on {@link org.apache.wicket.pageStore.AsynchronousDataStore} validating + * that the used data structures can be used simultaneously. + * + * @throws Exception + */ + @Test + public void randomOperations() throws Exception + { + ExecutorService executorService = Executors.newFixedThreadPool(50); + + for (int i = 0; i < EXECUTIONS; i++) + { + Runnable task = TASKS[RND.nextInt(TASKS.length)]; + executorService.submit(task); + } + LATCH.await(); + executorService.shutdown(); + } + + private static abstract class AbstractTask implements Runnable + { + /** the ids for the stored/removed pages */ + private static final int[] PAGE_IDS = new int[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 }; + + protected final PageWindowManager pageWindowManager; + + private AbstractTask(PageWindowManager pageWindowManager) + { + this.pageWindowManager = pageWindowManager; + } + + protected abstract void r(); + + public void run() + { + try + { + r(); + } + finally + { + LATCH.countDown(); + } + } + + protected int getPageId() + { + return PAGE_IDS[RND.nextInt(PAGE_IDS.length)]; + } + } + + private static class CreatePageWindowTask extends AbstractTask + { + private CreatePageWindowTask(PageWindowManager pageWindowManager) + { + super(pageWindowManager); + } + + @Override + public void r() + { + pageWindowManager.createPageWindow(getPageId(), 1000); + } + } + + private static class GetPageWindowTask extends AbstractTask + { + private GetPageWindowTask(PageWindowManager pageWindowManager) + { + super(pageWindowManager); + } + + @Override + public void r() + { + pageWindowManager.getPageWindow(getPageId()); + } + } + + private static class RemovePageInSessionTask extends AbstractTask + { + private RemovePageInSessionTask(PageWindowManager pageWindowManager) + { + super(pageWindowManager); + } + + @Override + public void r() + { + pageWindowManager.removePage(getPageId()); + } + } } http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-core/src/test/java/org/apache/wicket/pageStore/AsynchronousDataStoreTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/AsynchronousDataStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/AsynchronousDataStoreTest.java index 7d2d4c1..50a35b4 100644 --- a/wicket-core/src/test/java/org/apache/wicket/pageStore/AsynchronousDataStoreTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/AsynchronousDataStoreTest.java @@ -29,8 +29,11 @@ import org.junit.Test; */ public class AsynchronousDataStoreTest { +// private static final IDataStore WRAPPED_DATA_STORE = new DiskDataStore("asyncDataStoreApp", new StoreSettings(null).getFileStoreFolder(), Bytes.kilobytes(1)); + private static final IDataStore WRAPPED_DATA_STORE = new InMemoryPageStore(); + /** the data store under test */ - private static IDataStore DATA_STORE = new AsynchronousDataStore(new InMemoryPageStore(), 100); + private static final IDataStore DATA_STORE = new AsynchronousDataStore(WRAPPED_DATA_STORE, 100); /** the data for each page */ private static final byte[] DATA = new byte[] { 1, 2, 3 }; @@ -76,7 +79,6 @@ public class AsynchronousDataStoreTest private static abstract class AbstractTask implements Runnable { - protected abstract void r(); public void run() http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-core/src/test/java/org/apache/wicket/pageStore/DiskDataStoreTest.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/DiskDataStoreTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/DiskDataStoreTest.java index 5e42f64..1de69f4 100644 --- a/wicket-core/src/test/java/org/apache/wicket/pageStore/DiskDataStoreTest.java +++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/DiskDataStoreTest.java @@ -52,7 +52,6 @@ public class DiskDataStoreTest extends Assert private static final int FILE_SIZE_MIN = 1024 * 200; private static final int FILE_SIZE_MAX = 1024 * 300; private static final Bytes MAX_SIZE_PER_SESSION = Bytes.megabytes(10); - private static final int FILE_CHANNEL_POOL_CAPACITY = 100; private static final int SESSION_COUNT = 50; private static final int FILES_COUNT = 1000; private static final int SLEEP_MAX = 10; http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java b/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java index 64a8a9d..0087666 100644 --- a/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java +++ b/wicket-core/src/test/java/org/apache/wicket/versioning/InMemoryPageStore.java @@ -18,6 +18,7 @@ package org.apache.wicket.versioning; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import org.apache.wicket.pageStore.IDataStore; @@ -40,7 +41,7 @@ public class InMemoryPageStore implements IDataStore */ public InMemoryPageStore() { - store = new HashMap<String, Map<Integer, byte[]>>(); + store = new ConcurrentHashMap<String, Map<Integer, byte[]>> (); } public void destroy() @@ -92,7 +93,7 @@ public class InMemoryPageStore implements IDataStore Map<Integer, byte[]> sessionPages = store.get(sessionId); if (sessionPages == null) { - sessionPages = new HashMap<Integer, byte[]>(); + sessionPages = new ConcurrentHashMap<Integer, byte[]>(); store.put(sessionId, sessionPages); } http://git-wip-us.apache.org/repos/asf/wicket/blob/c6f7651c/wicket-util/src/main/java/org/apache/wicket/util/file/Files.java ---------------------------------------------------------------------- diff --git a/wicket-util/src/main/java/org/apache/wicket/util/file/Files.java b/wicket-util/src/main/java/org/apache/wicket/util/file/Files.java index f9d2579..4e89a7b 100644 --- a/wicket-util/src/main/java/org/apache/wicket/util/file/Files.java +++ b/wicket-util/src/main/java/org/apache/wicket/util/file/Files.java @@ -461,6 +461,7 @@ public class Files try { Thread.sleep(100); + if (folder.exists()) return true; } catch (InterruptedException ix) {
