WW-5414 Always call afterInvocation even in case of exception

apache · May 11, 2024 · 0141068 · 0141068
1 parent 649760d
commit 0141068
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 4 deletions.
diff --git a/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java b/core/src/main/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcess.java
@@ -20,6 +20,8 @@
 
 import com.opensymphony.xwork2.ActionContext;
 import com.opensymphony.xwork2.ActionInvocation;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.io.Serializable;
 
@@ -30,6 +32,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable
 
     private static final long serialVersionUID = 3884464776311686443L;
 
+    private static final Logger LOG = LogManager.getLogger(StrutsBackgroundProcess.class);
+
     private final String threadName;
     private final int threadPriority;
 
@@ -44,8 +48,8 @@ public class StrutsBackgroundProcess implements BackgroundProcess, Serializable
     /**
      * Constructs a background process
      *
-     * @param invocation The action invocation
-     * @param threadName The name of background thread
+     * @param invocation     The action invocation
+     * @param threadName     The name of background thread
      * @param threadPriority The priority of background thread
      */
     public StrutsBackgroundProcess(ActionInvocation invocation, String threadName, int threadPriority) {
@@ -61,11 +65,15 @@ public BackgroundProcess prepare() {
                 try {
                     beforeInvocation();
                     result = invocation.invokeActionOnly();
-                    afterInvocation();
                 } catch (Exception e) {
+                    try {
+                        afterInvocation();
+                    } catch (Exception ex) {
+                        LOG.warn("Exception during afterInvocation() execution", ex);
+                    }
                     exception = e;
                 } finally {
-                  done = true;
+                    done = true;
                 }
             });
             processThread.setName(threadName);

diff --git a/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java b/core/src/test/java/org/apache/struts2/interceptor/exec/StrutsBackgroundProcessTest.java
@@ -120,6 +120,19 @@ public void testMultipleProcesses() throws InterruptedException {
         assertEquals(100, mutableState.get());
     }
 
+    public void testErrorableProcesses() throws InterruptedException {
+        MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> {
+            throw new IllegalStateException("boom");
+        });
+
+        BackgroundProcess bp = new ErrorableBackgroundProcess(invocation, "error").prepare();
+        executor.execute(bp);
+
+        Thread.sleep(100);
+
+        assertTrue("afterInvocation not called in case of exception", ((ErrorableBackgroundProcess) bp).isDoneAfter());
+    }
+
     public void testUnpreparedProcess() throws ExecutionException, InterruptedException, TimeoutException {
         // given
         MockActionInvocationWithActionInvoker invocation = new MockActionInvocationWithActionInvoker(() -> "done");
@@ -177,3 +190,22 @@ protected void afterInvocation() throws Exception {
         lock.notify();
     }
 }
+
+class ErrorableBackgroundProcess extends StrutsBackgroundProcess {
+
+    private boolean doneAfter;
+
+    public ErrorableBackgroundProcess(ActionInvocation invocation, String name) {
+        super(invocation, name, Thread.NORM_PRIORITY);
+    }
+
+    @Override
+    protected void afterInvocation() throws Exception {
+        super.afterInvocation();
+        doneAfter = true;
+    }
+
+    public boolean isDoneAfter() {
+        return doneAfter;
+    }
+}