Repository: wicket Updated Branches: refs/heads/master 6bda62d63 -> 3398764a7
[WICKET-6603] Asypc page/data store destroyed without hanging Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/3398764a Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/3398764a Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/3398764a Branch: refs/heads/master Commit: 3398764a77a6bb92d4a8859c49280b8ede77a708 Parents: 6bda62d Author: Maxim Solodovnik <solomax...@gmail.com> Authored: Fri Oct 26 22:56:06 2018 +0700 Committer: Maxim Solodovnik <solomax...@gmail.com> Committed: Fri Oct 26 22:56:06 2018 +0700 ---------------------------------------------------------------------- .../wicket/pageStore/AsynchronousDataStore.java | 61 ++++++++----------- .../wicket/pageStore/AsynchronousPageStore.java | 62 ++++++++------------ 2 files changed, 52 insertions(+), 71 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/3398764a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java index 7c46a7a..f749354 100644 --- a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java +++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousDataStore.java @@ -38,7 +38,7 @@ import org.slf4j.LoggerFactory; * It starts only one instance of {@link PageSavingRunnable} because all we need is to make the page * storing asynchronous. We don't want to write concurrently in the wrapped {@link IDataStore}, * though it may happen in the extreme case when the queue is full. These cases should be avoided. - * + * * @author Matej Knopp */ public class AsynchronousDataStore implements IDataStore @@ -59,7 +59,7 @@ public class AsynchronousDataStore implements IDataStore /** * The page saving thread. */ - private final Thread pageSavingThread; + private Thread pageSavingThread; /** * The wrapped {@link IDataStore} that actually stores that pages @@ -79,7 +79,7 @@ public class AsynchronousDataStore implements IDataStore /** * Construct. - * + * * @param dataStore * the wrapped {@link IDataStore} that actually saved the data * @param capacity @@ -91,8 +91,7 @@ public class AsynchronousDataStore implements IDataStore entries = new LinkedBlockingQueue<>(capacity); entryMap = new ConcurrentHashMap<>(); - PageSavingRunnable savingRunnable = new PageSavingRunnable(dataStore, entries, entryMap); - pageSavingThread = new Thread(savingRunnable, "Wicket-AsyncDataStore-PageSavingThread"); + pageSavingThread = new Thread(new PageSavingRunnable(), "Wicket-AsyncDataStore-PageSavingThread"); pageSavingThread.setDaemon(true); pageSavingThread.start(); } @@ -100,12 +99,13 @@ public class AsynchronousDataStore implements IDataStore @Override public void destroy() { - if (pageSavingThread.isAlive()) + final Thread thread = pageSavingThread; + pageSavingThread = null; + if (thread != null && thread.isAlive()) { - pageSavingThread.interrupt(); try { - pageSavingThread.join(); + thread.join(); } catch (InterruptedException e) { log.error(e.getMessage(), e); @@ -117,7 +117,7 @@ public class AsynchronousDataStore implements IDataStore /** * Little helper - * + * * @param sessionId * @param id * @return Entry @@ -192,12 +192,16 @@ public class AsynchronousDataStore implements IDataStore /** * Save the entry in the queue if there is a room or directly pass it to the wrapped * {@link IDataStore} if there is no such - * + * * @see org.apache.wicket.pageStore.IDataStore#storeData(java.lang.String, int, byte[]) */ @Override public void storeData(final String sessionId, final int id, final byte[] data) { + if (pageSavingThread == null) + { + return; + } Entry entry = new Entry(sessionId, id, data); String key = getKey(entry); entryMap.put(key, entry); @@ -216,13 +220,16 @@ public class AsynchronousDataStore implements IDataStore catch (InterruptedException e) { log.error(e.getMessage(), e); - entryMap.remove(key); - dataStore.storeData(sessionId, id, data); + if (pageSavingThread != null) + { + entryMap.remove(key); + dataStore.storeData(sessionId, id, data); + } } } /** - * + * * @param pageId * @param sessionId * @return generated key @@ -233,7 +240,7 @@ public class AsynchronousDataStore implements IDataStore } /** - * + * * @param entry * @return generated key */ @@ -301,28 +308,12 @@ public class AsynchronousDataStore implements IDataStore /** * The thread that acts as consumer of {@link Entry}ies */ - private static class PageSavingRunnable implements Runnable + private class PageSavingRunnable implements Runnable { - private static final Logger log = LoggerFactory.getLogger(PageSavingRunnable.class); - - private final BlockingQueue<Entry> entries; - - private final ConcurrentMap<String, Entry> entryMap; - - private final IDataStore dataStore; - - private PageSavingRunnable(IDataStore dataStore, BlockingQueue<Entry> entries, - ConcurrentMap<String, Entry> entryMap) - { - this.dataStore = dataStore; - this.entries = entries; - this.entryMap = entryMap; - } - @Override public void run() { - while (!Thread.interrupted()) + while (pageSavingThread != null) { Entry entry = null; try @@ -331,12 +322,12 @@ public class AsynchronousDataStore implements IDataStore } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + log.debug("PageSavingRunnable:: Interrupted..."); } - if (entry != null) + if (entry != null && pageSavingThread != null) { - log.debug("Saving asynchronously: {}...", entry); + log.debug("PageSavingRunnable:: Saving asynchronously: {}...", entry); dataStore.storeData(entry.sessionId, entry.pageId, entry.data); entryMap.remove(getKey(entry)); } http://git-wip-us.apache.org/repos/asf/wicket/blob/3398764a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousPageStore.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousPageStore.java index 43993ba..c24b2a0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousPageStore.java +++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/AsynchronousPageStore.java @@ -39,9 +39,9 @@ import org.slf4j.LoggerFactory; * It starts only one instance of {@link PageSavingRunnable} because all we need is to make the page * storing asynchronous. We don't want to write concurrently in the wrapped {@link IPageStore}, * though it may happen in the extreme case when the queue is full. These cases should be avoided. - * + * * Based on AsynchronousDataStore (@author Matej Knopp). - * + * * @author manuelbarzi */ public class AsynchronousPageStore implements IPageStore @@ -63,7 +63,7 @@ public class AsynchronousPageStore implements IPageStore /** * The page saving thread. */ - private final Thread pageSavingThread; + private Thread pageSavingThread; /** * The wrapped {@link IPageStore} that actually stores that pages @@ -83,7 +83,7 @@ public class AsynchronousPageStore implements IPageStore /** * Construct. - * + * * @param delegate * the wrapped {@link IPageStore} that actually saved the page * @param capacity @@ -95,15 +95,14 @@ public class AsynchronousPageStore implements IPageStore entries = new LinkedBlockingQueue<>(capacity); entryMap = new ConcurrentHashMap<>(); - PageSavingRunnable savingRunnable = new PageSavingRunnable(delegate, entries, entryMap); - pageSavingThread = new Thread(savingRunnable, "Wicket-AsyncPageStore-PageSavingThread"); + pageSavingThread = new Thread(new PageSavingRunnable(), "Wicket-AsyncPageStore-PageSavingThread"); pageSavingThread.setDaemon(true); pageSavingThread.start(); } /** * Little helper - * + * * @param sessionId * @param pageId * @return Entry @@ -114,7 +113,7 @@ public class AsynchronousPageStore implements IPageStore } /** - * + * * @param pageId * @param sessionId * @return generated key @@ -125,7 +124,7 @@ public class AsynchronousPageStore implements IPageStore } /** - * + * * @param entry * @return generated key */ @@ -186,28 +185,12 @@ public class AsynchronousPageStore implements IPageStore /** * The thread that acts as consumer of {@link Entry}ies */ - private static class PageSavingRunnable implements Runnable + private class PageSavingRunnable implements Runnable { - private static final Logger log = LoggerFactory.getLogger(PageSavingRunnable.class); - - private final BlockingQueue<Entry> entries; - - private final ConcurrentMap<String, Entry> entryMap; - - private final IPageStore delegate; - - private PageSavingRunnable(IPageStore delegate, BlockingQueue<Entry> entries, - ConcurrentMap<String, Entry> entryMap) - { - this.delegate = delegate; - this.entries = entries; - this.entryMap = entryMap; - } - @Override public void run() { - while (!Thread.interrupted()) + while (pageSavingThread != null) { Entry entry = null; try @@ -216,12 +199,12 @@ public class AsynchronousPageStore implements IPageStore } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + log.debug("PageSavingRunnable:: Interrupted..."); } - if (entry != null) + if (entry != null && pageSavingThread != null) { - log.debug("Saving asynchronously: {}...", entry); + log.debug("PageSavingRunnable:: Saving asynchronously: {}...", entry); delegate.storePage(entry.sessionId, entry.page); entryMap.remove(getKey(entry)); } @@ -232,19 +215,19 @@ public class AsynchronousPageStore implements IPageStore @Override public void destroy() { - if (pageSavingThread.isAlive()) + final Thread thread = pageSavingThread; + pageSavingThread = null; + if (thread != null && thread.isAlive()) { - pageSavingThread.interrupt(); try { - pageSavingThread.join(); + thread.join(); } catch (InterruptedException e) { log.error(e.getMessage(), e); } } - delegate.destroy(); } @@ -286,6 +269,10 @@ public class AsynchronousPageStore implements IPageStore @Override public void storePage(String sessionId, IManageablePage page) { + if (pageSavingThread == null) + { + return; + } Entry entry = new Entry(sessionId, page); String key = getKey(entry); entryMap.put(key, entry); @@ -308,8 +295,11 @@ public class AsynchronousPageStore implements IPageStore catch (InterruptedException e) { log.error(e.getMessage(), e); - entryMap.remove(key); - delegate.storePage(sessionId, page); + if (pageSavingThread != null) + { + entryMap.remove(key); + delegate.storePage(sessionId, page); + } } }
