[wicket] branch master updated: WICKET-6966, WICKET-7013: prevent unsynchronized access to pages (#587)

[mg...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

mgrigorov pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git


The following commit(s) were added to refs/heads/master by this push:
     new 125fe8ab96 WICKET-6966, WICKET-7013: prevent unsynchronized access to pages (#587)
125fe8ab96 is described below

commit 125fe8ab96ce54565aaadaa3ba911b57439b85e1
Author: dr0ps <32...@users.noreply.github.com>
AuthorDate: Thu May 4 20:37:05 2023 +0200

    WICKET-6966, WICKET-7013: prevent unsynchronized access to pages (#587)
    
    * Wicket-6966: prevent unsynchronized access to pages
    
    * Added concurrency test case
    
    ---------
    
    Co-authored-by: dr0ps <dr...@users.noreply.github.com>
---
 .../wicket/pageStore/InSessionPageStore.java       |   2 +-
 .../InSessionPageStoreConcurrencyTest.java         | 131 +++++++++++++++++++++
 2 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/wicket-core/src/main/java/org/apache/wicket/pageStore/InSessionPageStore.java b/wicket-core/src/main/java/org/apache/wicket/pageStore/InSessionPageStore.java
index b8c432a558..54d2fa8a9c 100644
--- a/wicket-core/src/main/java/org/apache/wicket/pageStore/InSessionPageStore.java
+++ b/wicket-core/src/main/java/org/apache/wicket/pageStore/InSessionPageStore.java
@@ -261,7 +261,7 @@ public class InSessionPageStore implements IPageStore
 		/**
 		 * Serialize pages before writing to output.
 		 */
-		private void writeObject(final ObjectOutputStream output) throws IOException
+		private synchronized void writeObject(final ObjectOutputStream output) throws IOException
 		{
 			// handle non-serialized pages
 			for (int p = 0; p < pages.size(); p++)
diff --git a/wicket-core/src/test/java/org/apache/wicket/pageStore/InSessionPageStoreConcurrencyTest.java b/wicket-core/src/test/java/org/apache/wicket/pageStore/InSessionPageStoreConcurrencyTest.java
new file mode 100644
index 0000000000..75594096e2
--- /dev/null
+++ b/wicket-core/src/test/java/org/apache/wicket/pageStore/InSessionPageStoreConcurrencyTest.java
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.wicket.pageStore;
+
+import org.apache.commons.io.output.NullOutputStream;
+import org.apache.wicket.IPageManagerProvider;
+import org.apache.wicket.mock.MockApplication;
+import org.apache.wicket.page.PageManager;
+import org.apache.wicket.protocol.http.WebApplication;
+import org.apache.wicket.protocol.http.mock.MockHttpSession;
+import org.apache.wicket.protocol.http.mock.MockServletContext;
+import org.apache.wicket.serialize.java.JavaSerializer;
+import org.apache.wicket.util.tester.WicketTester;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+public class InSessionPageStoreConcurrencyTest
+{
+	public static class PageStoringWicketTester extends WicketTester
+	{
+		final static InSessionPageStore session = new InSessionPageStore(10, new JavaSerializer("key"));
+
+		public PageStoringWicketTester(final WebApplication application, final boolean init)
+		{
+			super(application, init);
+		}
+
+		@Override
+		protected IPageManagerProvider newTestPageManagerProvider()
+		{
+			return () ->
+					new PageManager(session);
+		}
+	}
+
+	@Test
+	public void testConcurrentModifications() throws Exception
+	{
+		final WebApplication application = new MockApplication();
+
+		final MockHttpSession httpSession = new MockHttpSession(new MockServletContext(application, ""));
+
+		//prepare servlet context
+		final WicketTester wicketTester = new PageStoringWicketTester(application, true)
+		{
+			@Override
+			public MockHttpSession getHttpSession()
+			{
+				return httpSession;
+			}
+		};
+		wicketTester.executeUrl("/");
+
+
+		ExecutorService executor = Executors.newFixedThreadPool(1000);
+
+		Callable<Void> executeURLTask = () -> {
+			final WicketTester parallelWicketTester;
+			synchronized (application)
+			{
+				parallelWicketTester = new PageStoringWicketTester(application, false)
+				{
+					@Override
+					public MockHttpSession getHttpSession()
+					{
+						return httpSession;
+					}
+				};
+			}
+
+			parallelWicketTester.executeUrl("/");
+			return null;
+		};
+
+		Callable<Void> serializeSessionTask = () -> {
+			final NullOutputStream nullOutputStream = NullOutputStream.NULL_OUTPUT_STREAM;
+			try(final ObjectOutputStream objectOutputStream = new ObjectOutputStream(nullOutputStream))
+			{
+				objectOutputStream.writeObject(wicketTester.getSession());
+			}
+			catch(final IOException e)
+			{
+				throw new RuntimeException(e);
+			}
+			return null;
+		};
+
+		var tasks = Stream.concat(
+				IntStream.rangeClosed(1, 500).mapToObj(i -> serializeSessionTask),
+				IntStream.rangeClosed(1, 500).mapToObj(i -> executeURLTask)
+		).collect(Collectors.toList());
+
+		var futures = executor.invokeAll(tasks);
+
+		futures.forEach(
+				exceptionFuture -> {
+					try
+					{
+						exceptionFuture.get();
+					}
+					catch (InterruptedException| ExecutionException e)
+					{
+						throw new RuntimeException(e);
+					}
+				});
+	}
+
+}