Repository: wicket Updated Branches: refs/heads/wicket-7.x dc6d94879 -> b25984d4e
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/b25984d4 Tree: http://git-wip-us.apache.org/repos/asf/wicket/tree/b25984d4 Diff: http://git-wip-us.apache.org/repos/asf/wicket/diff/b25984d4 Branch: refs/heads/wicket-7.x Commit: b25984d4e9b9d07c5bf2d3e50c90bbb165554409 Parents: dc6d948 Author: Sven Meier <[email protected]> Authored: Wed Sep 13 23:44:53 2017 +0200 Committer: Sven Meier <[email protected]> Committed: Thu Sep 14 00:08:05 2017 +0200 ---------------------------------------------------------------------- .../apache/wicket/page/PageStoreManager.java | 56 ++++++++++++++------ 1 file changed, 40 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/wicket/blob/b25984d4/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 6934e0d..eb999ee 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. @@ -291,6 +299,14 @@ public class PageStoreManager extends AbstractPageManager ClassNotFoundException { s.defaultReadObject(); + + storingTouchedPages = new ThreadLocal<Boolean>() + { + protected Boolean initialValue() + { + return Boolean.FALSE; + }; + }; afterReadObject = new ArrayList<>(); @@ -317,15 +333,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; } @@ -425,11 +440,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); + } } } }
