[ https://issues.apache.org/jira/browse/WW-4900?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16289253#comment-16289253 ]
ASF GitHub Bot commented on WW-4900: ------------------------------------ lukaszlenart closed pull request #191: WW-4900 Makes BackgroundProcess transient URL: https://github.com/apache/struts/pull/191 This is a PR merged from a forked repository. As GitHub hides the original diff on merge, it is displayed below for the sake of provenance: As this is a foreign pull request (from a fork), the diff is supplied below (as it won't show otherwise due to GitHub magic): 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..cc30f8ac4 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 @@ 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; /** @@ -46,7 +47,6 @@ */ public 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() { @@ -96,7 +96,7 @@ protected void afterInvocation() throws Exception { * @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 @@ protected String doIntercept(ActionInvocation actionInvocation) throws Exception 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..b811b1dd5 --- /dev/null +++ b/core/src/test/java/org/apache/struts2/interceptor/BackgroundProcessTest.java @@ -0,0 +1,104 @@ +/* + * 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.mockobjects.servlet.MockHttpServletRequest; +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; +import java.util.concurrent.Callable; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; + +/** + * Test case for BackgroundProcessTest. + */ +public class BackgroundProcessTest extends StrutsInternalTestCase { + + public void testSerializeDeserialize() throws Exception { + final NotSerializableException expectedException = new NotSerializableException(new MockHttpServletRequest()); + final Semaphore lock = new Semaphore(1); + lock.acquire(); + MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(new Callable<String>() { + @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.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(); + 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); + } + + + private class MockActionInvocationWithActionInvoker extends MockActionInvocation { + private Callable<String> actionInvoker; + + MockActionInvocationWithActionInvoker(Callable<String> 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 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.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 void testWaitDelayAndJobAlreadyDone2() throws Exception { 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); } ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org > NotSerializableException: > com.opensymphony.xwork2.inject.ContainerImpl$ConstructorInjector when using > ExecuteAndWait interceptor > -------------------------------------------------------------------------------------------------------------------------------- > > Key: WW-4900 > URL: https://issues.apache.org/jira/browse/WW-4900 > Project: Struts 2 > Issue Type: Bug > Affects Versions: 2.5.14.1 > Reporter: Erica Kane > Assignee: Yasser Zamani > Fix For: 2.5.15 > > > We are running Struts 2.5.14.1 and working on externalizing Tomcat session > state. This requires Serializable sessions. However, our Action with the > ExecuteAndWait interceptor fails. Since our original code was quite complex I > wrote a simpler one below which demonstrates the exact same behavior. > The simple action is shown here: > {noformat} > package com.sentrylink.web.actions; > import java.util.concurrent.TimeUnit; > import org.apache.struts2.convention.annotation.InterceptorRef; > import org.apache.struts2.convention.annotation.InterceptorRefs; > import org.apache.struts2.convention.annotation.Result; > import org.apache.struts2.convention.annotation.Results; > import com.opensymphony.xwork2.ActionSupport; > @SuppressWarnings("serial") > @Results({ > @Result(name="wait", location="/"), > @Result(name=ActionSupport.SUCCESS, > location="/WEB-INF/content/messagePage.jsp"), > }) > @InterceptorRefs({ > @InterceptorRef("webStack"), > @InterceptorRef("execAndWait") > }) > public class TestExecuteAndWait extends ActionSupport { > public String execute() throws Exception { > TimeUnit.SECONDS.sleep(10); > return SUCCESS; > } > } > {noformat} > Running this gives > {noformat} > WARNING: Cannot serialize session attribute __execWaittest-execute-and-wait > for session 74CDB9F8D00BBC697030AFC6978E94F6 > java.io.NotSerializableException: > com.opensymphony.xwork2.inject.ContainerImpl$ConstructorInjector > {noformat} > Removing the ExecuteAndWait interceptor fixes the issue. > According to [~yasser.zamani] in WW-4873 : I reviewed > {{ExecuteAndWaitInterceptor}} and seems has this bug when session goes to > being serialized in middle of an background process. -- This message was sent by Atlassian JIRA (v6.4.14#64029)