From dda3facc265d4e1c3b1c60fa2bcc4240953ee35c Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 13 Dec 2017 12:41:22 +0330 Subject: [PATCH 1/3] WW-4900 Makes BackgroundProcess transient --- .../interceptor/BackgroundProcess.java | 16 ++--- .../ExecuteAndWaitInterceptor.java | 6 ++ .../interceptor/BackgroundProcessTest.java | 65 +++++++++++++++++++ .../ExecuteAndWaitInterceptorTest.java | 39 +++++++++++ 4 files changed, 118 insertions(+), 8 deletions(-) create mode 100644 core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java diff --git a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java index 20b330eae..5210c4072 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java +++ b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java @@ -31,10 +31,11 @@ public class BackgroundProcess implements Serializable { private static final long serialVersionUID = 3884464776311686443L; - protected Object action; - protected ActionInvocation invocation; + //WW-4900 transient since 2.5.15 + transient protected ActionInvocation invocation; + transient protected Exception exception; + protected String result; - protected Exception exception; protected boolean done; /** @@ -44,9 +45,8 @@ public class BackgroundProcess implements Serializable { * @param invocation The action invocation * @param threadPriority The thread priority */ - public BackgroundProcess(String threadName, final ActionInvocation invocation, int threadPriority) { + BackgroundProcess(String threadName, final ActionInvocation invocation, int threadPriority) { this.invocation = invocation; - this.action = invocation.getAction(); try { final Thread t = new Thread(new Runnable() { public void run() { @@ -75,7 +75,7 @@ public class BackgroundProcess implements Serializable { * * @throws Exception any exception thrown will be thrown, in turn, by the ExecuteAndWaitInterceptor */ - protected void beforeInvocation() throws Exception { + private void beforeInvocation() throws Exception { ActionContext.setContext(invocation.getInvocationContext()); } @@ -86,7 +86,7 @@ public class BackgroundProcess implements Serializable { * * @throws Exception any exception thrown will be thrown, in turn, by the ExecuteAndWaitInterceptor */ - protected void afterInvocation() throws Exception { + private void afterInvocation() throws Exception { ActionContext.setContext(null); } @@ -96,7 +96,7 @@ public class BackgroundProcess implements Serializable { * @return the action. */ public Object getAction() { - return action; + return invocation.getAction(); } /** diff --git a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java index 2b3e063e0..b49ebcae7 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptor.java @@ -243,6 +243,12 @@ public class ExecuteAndWaitInterceptor extends MethodFilterInterceptor { synchronized (httpSession) { BackgroundProcess bp = (BackgroundProcess) session.get(KEY + name); + //WW-4900 Checks if from a de-serialized session? so background thread missed, let's start a new one. + if (bp != null && bp.getInvocation() == null) { + session.remove(KEY + name); + bp = null; + } + if ((!executeAfterValidationPass || secondTime) && bp == null) { bp = getNewBackgroundProcess(name, actionInvocation, threadPriority); session.put(KEY + name, bp); diff --git a/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java new file mode 100644 index 000000000..ceeda9e2a --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java @@ -0,0 +1,65 @@ +/* + * 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.struts2.interceptor; + +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.mock.MockActionInvocation; +import org.apache.struts2.StrutsInternalTestCase; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; + +/** + * Test case for BackgroundProcessTest. + */ +public class BackgroundProcessTest extends StrutsInternalTestCase { + + public void testSerializeDeserialize() throws Exception { + MockActionInvocation invocation = new MockActionInvocation(); + invocation.setResultCode("BackgroundProcessTest.testSerializeDeserialize"); + invocation.setInvocationContext(ActionContext.getContext()); + + BackgroundProcess bp = new BackgroundProcess("BackgroundProcessTest.testSerializeDeserialize", invocation + , Thread.MIN_PRIORITY); + + bp.exception = new Exception(); + bp.done = true; + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos); + oos.writeObject(bp); + oos.close(); + assertTrue("should have serialized data", baos.size() > 0); + byte b[] = baos.toByteArray(); + baos.close(); + + ByteArrayInputStream bais = new ByteArrayInputStream(b); + ObjectInputStream ois = new ObjectInputStream(bais); + BackgroundProcess deserializedBp = (BackgroundProcess) ois.readObject(); + ois.close(); + bais.close(); + + assertNull("invocation should not be serialized", deserializedBp.invocation); + assertNull("exception should not be serialized", deserializedBp.exception); + assertEquals(bp.result, deserializedBp.result); + assertEquals(bp.done, deserializedBp.done); + } +} \ No newline at end of file diff --git a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java index 9106a1b6a..c99cf0825 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ExecuteAndWaitInterceptorTest.java @@ -38,6 +38,10 @@ import org.apache.struts2.views.jsp.StrutsMockHttpServletRequest; import org.apache.struts2.views.jsp.StrutsMockHttpSession; import javax.servlet.http.HttpSession; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.ObjectInputStream; +import java.io.ObjectOutputStream; import java.util.HashMap; import java.util.Map; @@ -159,6 +163,41 @@ public class ExecuteAndWaitInterceptorTest extends StrutsInternalTestCase { assertTrue("Job done already after 500 so there should not be such long delay", diff <= 1000); } + public void testFromDeserializedSession() throws Exception { + waitInterceptor.setDelay(0); + waitInterceptor.setDelaySleepInterval(0); + + ActionProxy proxy = buildProxy("action1"); + String result = proxy.execute(); + assertEquals("wait", result); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + ObjectOutputStream oos = new ObjectOutputStream(baos); + oos.writeObject(session);//WW-4900 action1 and invocation are not serializable but we should not fail at this line + oos.close(); + byte b[] = baos.toByteArray(); + baos.close(); + + ByteArrayInputStream bais = new ByteArrayInputStream(b); + ObjectInputStream ois = new ObjectInputStream(bais); + session = (Map) ois.readObject(); + context.put(ActionContext.SESSION, session); + ois.close(); + bais.close(); + + Thread.sleep(1000); + + ActionProxy proxy2 = buildProxy("action1"); + String result2 = proxy2.execute(); + assertEquals("wait", result2);//WW-4900 A new thread should be started when background thread missed + + Thread.sleep(1000); + + ActionProxy proxy3 = buildProxy("action1"); + String result3 = proxy3.execute(); + assertEquals("success", result3); + } + protected ActionProxy buildProxy(String actionName) throws Exception { return actionProxyFactory.createActionProxy("", actionName, null, context); } From 6b131550a649106766f45c83279b036544afc58c Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 13 Dec 2017 13:11:35 +0330 Subject: [PATCH 2/3] WW-4900 Reverts BackgroundProcess access modifiers --- .../org/apache/struts2/interceptor/BackgroundProcess.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java index 5210c4072..cc30f8ac4 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java +++ b/core/src/main/java/org/apache/struts2/interceptor/BackgroundProcess.java @@ -45,7 +45,7 @@ public class BackgroundProcess implements Serializable { * @param invocation The action invocation * @param threadPriority The thread priority */ - BackgroundProcess(String threadName, final ActionInvocation invocation, int threadPriority) { + public BackgroundProcess(String threadName, final ActionInvocation invocation, int threadPriority) { this.invocation = invocation; try { final Thread t = new Thread(new Runnable() { @@ -75,7 +75,7 @@ public class BackgroundProcess implements Serializable { * * @throws Exception any exception thrown will be thrown, in turn, by the ExecuteAndWaitInterceptor */ - private void beforeInvocation() throws Exception { + protected void beforeInvocation() throws Exception { ActionContext.setContext(invocation.getInvocationContext()); } @@ -86,7 +86,7 @@ public class BackgroundProcess implements Serializable { * * @throws Exception any exception thrown will be thrown, in turn, by the ExecuteAndWaitInterceptor */ - private void afterInvocation() throws Exception { + protected void afterInvocation() throws Exception { ActionContext.setContext(null); } From a8ecd9bd297670b048dd3d12ee7211d4984db664 Mon Sep 17 00:00:00 2001 From: Yasser Zamani Date: Wed, 13 Dec 2017 14:58:54 +0330 Subject: [PATCH 3/3] WW-4900 Fixes BackgroundProcessTest via synchronization --- .../interceptor/BackgroundProcessTest.java | 47 +++++++++++++++++-- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java index ceeda9e2a..b811b1dd5 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java @@ -18,6 +18,7 @@ */ package org.apache.struts2.interceptor; +import com.mockobjects.servlet.MockHttpServletRequest; import com.opensymphony.xwork2.ActionContext; import com.opensymphony.xwork2.mock.MockActionInvocation; import org.apache.struts2.StrutsInternalTestCase; @@ -26,6 +27,9 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.util.concurrent.Callable; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; /** * Test case for BackgroundProcessTest. @@ -33,21 +37,35 @@ import java.io.ObjectOutputStream; public class BackgroundProcessTest extends StrutsInternalTestCase { public void testSerializeDeserialize() throws Exception { - MockActionInvocation invocation = new MockActionInvocation(); - invocation.setResultCode("BackgroundProcessTest.testSerializeDeserialize"); + final NotSerializableException expectedException = new NotSerializableException(new MockHttpServletRequest()); + final Semaphore lock = new Semaphore(1); + lock.acquire(); + MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(new Callable() { + @Override + public String call() throws Exception { + lock.release(); + throw expectedException; + } + }); invocation.setInvocationContext(ActionContext.getContext()); BackgroundProcess bp = new BackgroundProcess("BackgroundProcessTest.testSerializeDeserialize", invocation , Thread.MIN_PRIORITY); + if(!lock.tryAcquire(1500L, TimeUnit.MILLISECONDS)) { + lock.release(); + fail("background thread did not release lock on timeout"); + } + lock.release(); - bp.exception = new Exception(); + bp.result = "BackgroundProcessTest.testSerializeDeserialize"; bp.done = true; + Thread.sleep(1000);//give a chance to background thread to set exception + assertEquals(expectedException, bp.exception); ByteArrayOutputStream baos = new ByteArrayOutputStream(); ObjectOutputStream oos = new ObjectOutputStream(baos); oos.writeObject(bp); oos.close(); - assertTrue("should have serialized data", baos.size() > 0); byte b[] = baos.toByteArray(); baos.close(); @@ -62,4 +80,25 @@ public class BackgroundProcessTest extends StrutsInternalTestCase { assertEquals(bp.result, deserializedBp.result); assertEquals(bp.done, deserializedBp.done); } + + + private class MockActionInvocationWithActionInvoker extends MockActionInvocation { + private Callable actionInvoker; + + MockActionInvocationWithActionInvoker(Callable actionInvoker){ + this.actionInvoker = actionInvoker; + } + + @Override + public String invokeActionOnly() throws Exception { + return actionInvoker.call(); + } + } + + private class NotSerializableException extends Exception { + private MockHttpServletRequest notSerializableField; + NotSerializableException(MockHttpServletRequest notSerializableField) { + this.notSerializableField = notSerializableField; + } + } } \ No newline at end of file