Repository: wicket Updated Branches: refs/heads/master 72a5d312a -> 68b24aa78
WICKET-6465 prevent unbound during storeTouchedPages with ThreadLocal This closes #233 Project: http://git-wip-us.apache.org/repos/asf/wicket/repo Commit: http://git-wip-us.apache.org/repos/asf/wicket/commit/68b24aa7 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/68b24aa7 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/68b24aa7 Branch: refs/heads/master Commit: 68b24aa788b25a67cfb16bee08a85328dad7b91c Parents: 72a5d31 Author: Sven Meier <[email protected]> Authored: Wed Sep 13 23:40:14 2017 +0200 Committer: Sven Meier <[email protected]> Committed: Thu Sep 14 00:10:01 2017 +0200 ---------------------------------------------------------------------- .../apache/wicket/page/PageStoreManager.java | 63 ++++++++++++++------ 1 file changed, 44 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/68b24aa7/wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java ---------------------------------------------------------------------- diff --git a/wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java b/wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java index ae68138..2db301f 100644 --- a/wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java +++ b/wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java @@ -24,7 +24,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.concurrent.atomic.AtomicBoolean; import javax.servlet.http.HttpSessionBindingEvent; import javax.servlet.http.HttpSessionBindingListener; @@ -65,8 +64,8 @@ public class PageStoreManager extends AbstractPageManager if (MANAGERS.containsKey(applicationName)) { - throw new IllegalStateException("Manager for application with key '" + applicationName - + "' already exists."); + throw new IllegalStateException( + "Manager for application with key '" + applicationName + "' already exists."); } MANAGERS.put(applicationName, this); } @@ -95,14 +94,23 @@ public class PageStoreManager extends AbstractPageManager private transient List<Object> afterReadObject; /** - * A flag indicating whether this session entry has been re-set in the Session. - * Web containers intercept {@link javax.servlet.http.HttpSession#setAttribute(String, Object)} - * to detect changes and replicate the session. If the attribute has been already - * bound in the session then it will be first unbound and then re-bound again. - * This flag helps us to detect <em>update</em> operations and skip the default behavior - * of {@link #valueUnbound(HttpSessionBindingEvent)}. + * A flag indicating whether this session entry is being re-set in the Session. + * <p> + * Web containers intercept + * {@link javax.servlet.http.HttpSession#setAttribute(String, Object)} to detect changes and + * replicate the session. If the attribute has been already bound in the session then + * {@link #valueUnbound(HttpSessionBindingEvent)} might get called - this flag + * helps us to ignore the invocation in that case. + * + * @see #valueUnbound(HttpSessionBindingEvent) */ - private final AtomicBoolean updating = new AtomicBoolean(false); + private transient ThreadLocal<Boolean> storingTouchedPages = new ThreadLocal<Boolean>() + { + protected Boolean initialValue() + { + return Boolean.FALSE; + }; + }; /** * Construct. @@ -300,11 +308,19 @@ public class PageStoreManager extends AbstractPageManager * @throws ClassNotFoundException */ @SuppressWarnings("unchecked") - private void readObject(final ObjectInputStream s) throws IOException, - ClassNotFoundException + private void readObject(final ObjectInputStream s) + throws IOException, ClassNotFoundException { s.defaultReadObject(); + storingTouchedPages = new ThreadLocal<Boolean>() + { + protected Boolean initialValue() + { + return Boolean.FALSE; + }; + }; + afterReadObject = new ArrayList<>(); List<Serializable> l = (List<Serializable>)s.readObject(); @@ -330,15 +346,14 @@ public class PageStoreManager extends AbstractPageManager @Override public void valueBound(HttpSessionBindingEvent event) { - updating.set(false); } @Override public void valueUnbound(HttpSessionBindingEvent event) { - if (updating.compareAndSet(true, false)) + if (storingTouchedPages.get()) { - // The entry has been updated. Do not remove the data + // triggered by #storeTouchedPages(), so do not remove the data return; } @@ -404,7 +419,8 @@ public class PageStoreManager extends AbstractPageManager } @Override - protected void removePage(final IManageablePage page) { + protected void removePage(final IManageablePage page) + { final SessionEntry sessionEntry = getSessionEntry(false); if (sessionEntry != null) { @@ -447,11 +463,20 @@ public class PageStoreManager extends AbstractPageManager entry.setSessionCache(touchedPages); for (IManageablePage page : touchedPages) { - // WICKET-5103 use the same sessionId as used in SessionEntry#getPage() + // WICKET-5103 use the same sessionId as used in + // SessionEntry#getPage() pageStore.storePage(entry.sessionId, page); } - entry.updating.set(true); - setSessionAttribute(getAttributeName(), entry); + + entry.storingTouchedPages.set(true); + try + { + setSessionAttribute(getAttributeName(), entry); + } + finally + { + entry.storingTouchedPages.set(false); + } } } }
