From 8b7a670821793a1bd15cea0fb388deaf88cd2d0e Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Wed, 16 Aug 2017 09:26:21 +0200 Subject: [PATCH] Cancel WebAsyncManager thread on request timeout Issue: SPR-15852 --- .../async/CallableInterceptorChain.java | 23 +++++++++++++++++ .../request/async/WebAsyncManager.java | 4 ++- .../async/WebAsyncManagerTimeoutTests.java | 25 +++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java index 3c81ee1a271..ac87f56d6d8 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/CallableInterceptorChain.java @@ -18,6 +18,7 @@ package org.springframework.web.context.request.async; import java.util.List; import java.util.concurrent.Callable; +import java.util.concurrent.Future; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -39,11 +40,19 @@ class CallableInterceptorChain { private int preProcessIndex = -1; + private volatile Future taskFuture; + public CallableInterceptorChain(List interceptors) { this.interceptors = interceptors; } + + public void setTaskFuture(Future taskFuture) { + this.taskFuture = taskFuture; + } + + public void applyBeforeConcurrentHandling(NativeWebRequest request, Callable task) throws Exception { for (CallableProcessingInterceptor interceptor : this.interceptors) { interceptor.beforeConcurrentHandling(request, task); @@ -77,6 +86,7 @@ class CallableInterceptorChain { } public Object triggerAfterTimeout(NativeWebRequest request, Callable task) { + cancelTask(); for (CallableProcessingInterceptor interceptor : this.interceptors) { try { Object result = interceptor.handleTimeout(request, task); @@ -94,7 +104,20 @@ class CallableInterceptorChain { return CallableProcessingInterceptor.RESULT_NONE; } + private void cancelTask() { + Future future = this.taskFuture; + if (future != null) { + try { + future.cancel(true); + } + catch (Throwable ex) { + // Ignore + } + } + } + public Object triggerAfterError(NativeWebRequest request, Callable task, Throwable throwable) { + cancelTask(); for (CallableProcessingInterceptor interceptor : this.interceptors) { try { Object result = interceptor.handleError(request, task, throwable); diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java index 420307809a0..0fac50969be 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/WebAsyncManager.java @@ -21,6 +21,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; +import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; import javax.servlet.http.HttpServletRequest; @@ -314,7 +315,7 @@ public final class WebAsyncManager { interceptorChain.applyBeforeConcurrentHandling(this.asyncWebRequest, callable); startAsyncProcessing(processingContext); try { - this.taskExecutor.submit(() -> { + Future future = this.taskExecutor.submit(() -> { Object result = null; try { interceptorChain.applyPreProcess(this.asyncWebRequest, callable); @@ -328,6 +329,7 @@ public final class WebAsyncManager { } setConcurrentResultAndDispatch(result); }); + interceptorChain.setTaskFuture(future); } catch (RejectedExecutionException ex) { Object result = interceptorChain.applyPostProcess(this.asyncWebRequest, callable, ex); diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java index 22a0e722809..7d6a506219d 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/WebAsyncManagerTimeoutTests.java @@ -17,6 +17,7 @@ package org.springframework.web.context.request.async; import java.util.concurrent.Callable; +import java.util.concurrent.Future; import javax.servlet.AsyncEvent; import org.junit.Before; @@ -30,9 +31,12 @@ import org.springframework.web.context.request.NativeWebRequest; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.mock; import static org.mockito.BDDMockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; import static org.springframework.web.context.request.async.CallableProcessingInterceptor.RESULT_NONE; /** @@ -151,6 +155,27 @@ public class WebAsyncManagerTimeoutTests { verify(interceptor).beforeConcurrentHandling(this.asyncWebRequest, callable); } + @SuppressWarnings("unchecked") + @Test + public void startCallableProcessingTimeoutAndCheckThreadInterrupted() throws Exception { + + StubCallable callable = new StubCallable(); + Future future = mock(Future.class); + + AsyncTaskExecutor executor = mock(AsyncTaskExecutor.class); + when(executor.submit(any(Runnable.class))).thenReturn(future); + + this.asyncManager.setTaskExecutor(executor); + this.asyncManager.startCallableProcessing(callable); + + this.asyncWebRequest.onTimeout(ASYNC_EVENT); + + assertTrue(this.asyncManager.hasConcurrentResult()); + + verify(future).cancel(true); + verifyNoMoreInteractions(future); + } + @Test public void startDeferredResultProcessingTimeoutAndComplete() throws Exception {