WICKET-6465 prevent unbound during storeTouchedPages only

wicket-core/src/main/java/org/apache/wicket/page/PageStoreManager.java

@@ -24,7 +24,6 @@
 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 PageStoreManager(final String applicationName, final IPageStore pageStore
 
 		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 @@ private static class SessionEntry implements Serializable, HttpSessionBindingLis
 		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 final ThreadLocal<Boolean> storingTouchedPages = new ThreadLocal<Boolean>()
+		{
+			protected Boolean initialValue()
+			{
+				return Boolean.FALSE;
+			};
+		};
 
 		/**
 		 * Construct.
@@ -300,16 +308,17 @@ private void writeObject(final ObjectOutputStream s) throws IOException
 		 * @throws ClassNotFoundException
 		 */
 		@SuppressWarnings("unchecked")
-		private void readObject(final ObjectInputStream s) throws IOException,
-			ClassNotFoundException
+		private void readObject(final ObjectInputStream s)
+			throws IOException, ClassNotFoundException
 		{
 			s.defaultReadObject();
 
 			afterReadObject = new ArrayList<>();
 
 			List<Serializable> l = (List<Serializable>)s.readObject();
 
-			// convert to temporary state after deserialization (will need to be processed
+			// convert to temporary state after deserialization (will need to be
+			// processed
 			// by convertAfterReadObject before the pages can be accessed)
 			IPageStore pageStore = getPageStore();
 			for (Serializable ser : l)
@@ -330,15 +339,14 @@ private void readObject(final ObjectInputStream s) throws IOException,
 		@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 +412,8 @@ protected IManageablePage getPage(int id)
 		}
 
 		@Override
-		protected void removePage(final IManageablePage page) {
+		protected void removePage(final IManageablePage page)
+		{
 			final SessionEntry sessionEntry = getSessionEntry(false);
 			if (sessionEntry != null)
 			{
@@ -447,11 +456,20 @@ protected void storeTouchedPages(final List<IManageablePage> touchedPages)
 				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);
+				}
 			}
 		}
 	}