diff --git a/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionAspectSupport.java b/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionAspectSupport.java index 993d2897eb3..9f6b3c20c63 100644 --- a/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionAspectSupport.java +++ b/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionAspectSupport.java @@ -20,6 +20,10 @@ import java.lang.reflect.Method; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; +import java.util.concurrent.Future; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanFactory; import org.springframework.beans.factory.BeanFactoryAware; @@ -28,11 +32,12 @@ import org.springframework.core.task.AsyncListenableTaskExecutor; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.core.task.support.TaskExecutorAdapter; import org.springframework.util.Assert; +import org.springframework.util.ReflectionUtils; import org.springframework.util.StringUtils; /** * Base class for asynchronous method execution aspects, such as - * {@link org.springframework.scheduling.annotation.AnnotationAsyncExecutionInterceptor} + * {@code org.springframework.scheduling.annotation.AnnotationAsyncExecutionInterceptor} * or {@code org.springframework.scheduling.aspectj.AnnotationAsyncExecutionAspect}. * *

Provides support for executor qualification on a method-by-method basis. @@ -42,14 +47,19 @@ import org.springframework.util.StringUtils; * * @author Chris Beams * @author Juergen Hoeller + * @author Stephane Nicoll * @since 3.1.2 */ public abstract class AsyncExecutionAspectSupport implements BeanFactoryAware { + protected final Log logger = LogFactory.getLog(getClass()); + private final Map executors = new ConcurrentHashMap(16); private Executor defaultExecutor; + private AsyncUncaughtExceptionHandler exceptionHandler; + private BeanFactory beanFactory; @@ -58,9 +68,18 @@ public abstract class AsyncExecutionAspectSupport implements BeanFactoryAware { * executor unless individual async methods indicate via qualifier that a more * specific executor should be used. * @param defaultExecutor the executor to use when executing asynchronous methods + * @param exceptionHandler the {@link AsyncUncaughtExceptionHandler} to use + */ + public AsyncExecutionAspectSupport(Executor defaultExecutor, AsyncUncaughtExceptionHandler exceptionHandler) { + this.defaultExecutor = defaultExecutor; + this.exceptionHandler = exceptionHandler; + } + + /** + * Create a new instance with a default {@link AsyncUncaughtExceptionHandler}. */ public AsyncExecutionAspectSupport(Executor defaultExecutor) { - this.defaultExecutor = defaultExecutor; + this(defaultExecutor, new SimpleAsyncUncaughtExceptionHandler()); } @@ -78,6 +97,14 @@ public abstract class AsyncExecutionAspectSupport implements BeanFactoryAware { this.defaultExecutor = defaultExecutor; } + /** + * Supply the {@link AsyncUncaughtExceptionHandler} to use to handle exceptions + * thrown by invoking asynchronous methods with a {@code void} return type. + */ + public void setExceptionHandler(AsyncUncaughtExceptionHandler exceptionHandler) { + this.exceptionHandler = exceptionHandler; + } + /** * Set the {@link BeanFactory} to be used when looking up executors by qualifier. */ @@ -125,4 +152,32 @@ public abstract class AsyncExecutionAspectSupport implements BeanFactoryAware { */ protected abstract String getExecutorQualifier(Method method); + /** + * Handles a fatal error thrown while asynchronously invoking the specified + * {@link Method}. + *

If the return type of the method is a {@link java.util.concurrent.Future} object, the original + * exception can be propagated by just throwing it at the higher level. However, + * for all other cases, the exception will not be transmitted back to the client. + * In that later case, the current {@link AsyncUncaughtExceptionHandler} will be + * used to manage such exception. + * @param ex the exception to handle + * @param method the method that was invoked + * @param params the parameters used to invoke the method + */ + protected void handleError(Throwable ex, Method method, Object... params) throws Exception { + if (method.getReturnType().isAssignableFrom(Future.class)) { + ReflectionUtils.rethrowException(ex); + } + else { + // Could not transmit the exception to the caller with default executor + try { + this.exceptionHandler.handleUncaughtException(ex, method, params); + } + catch (Throwable ex2) { + logger.error("Exception handler for async method '" + method.toGenericString() + + "' threw unexpected exception itself", ex2); + } + } + } + } diff --git a/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionInterceptor.java index ec7582f458f..1a89a40977d 100644 --- a/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/interceptor/AsyncExecutionInterceptor.java @@ -23,8 +23,6 @@ import java.util.concurrent.Future; import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.springframework.aop.support.AopUtils; import org.springframework.core.BridgeMethodResolver; @@ -32,7 +30,6 @@ import org.springframework.core.Ordered; import org.springframework.core.task.AsyncListenableTaskExecutor; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.util.ClassUtils; -import org.springframework.util.ReflectionUtils; import org.springframework.util.concurrent.ListenableFuture; /** @@ -70,11 +67,6 @@ import org.springframework.util.concurrent.ListenableFuture; public class AsyncExecutionInterceptor extends AsyncExecutionAspectSupport implements MethodInterceptor, Ordered { - private final Log logger = LogFactory.getLog(getClass()); - - private AsyncUncaughtExceptionHandler exceptionHandler; - - /** * Create a new {@code AsyncExecutionInterceptor}. * @param defaultExecutor the {@link Executor} (typically a Spring {@link AsyncTaskExecutor} @@ -82,27 +74,16 @@ public class AsyncExecutionInterceptor extends AsyncExecutionAspectSupport * @param exceptionHandler the {@link AsyncUncaughtExceptionHandler} to use */ public AsyncExecutionInterceptor(Executor defaultExecutor, AsyncUncaughtExceptionHandler exceptionHandler) { - super(defaultExecutor); - this.exceptionHandler = exceptionHandler; + super(defaultExecutor, exceptionHandler); } /** * Create a new instance with a default {@link AsyncUncaughtExceptionHandler}. */ public AsyncExecutionInterceptor(Executor defaultExecutor) { - this(defaultExecutor, new SimpleAsyncUncaughtExceptionHandler()); + super(defaultExecutor); } - - /** - * Supply the {@link AsyncUncaughtExceptionHandler} to use to handle exceptions - * thrown by invoking asynchronous methods with a {@code void} return type. - */ - public void setExceptionHandler(AsyncUncaughtExceptionHandler exceptionHandler) { - this.exceptionHandler = exceptionHandler; - } - - /** * Intercept the given method invocation, submit the actual calling of the method to * the correct task executor and return immediately to the caller. @@ -151,34 +132,6 @@ public class AsyncExecutionInterceptor extends AsyncExecutionAspectSupport } } - /** - * Handles a fatal error thrown while asynchronously invoking the specified - * {@link Method}. - *

If the return type of the method is a {@link Future} object, the original - * exception can be propagated by just throwing it at the higher level. However, - * for all other cases, the exception will not be transmitted back to the client. - * In that later case, the current {@link AsyncUncaughtExceptionHandler} will be - * used to manage such exception. - * @param ex the exception to handle - * @param method the method that was invoked - * @param params the parameters used to invoke the method - */ - protected void handleError(Throwable ex, Method method, Object... params) throws Exception { - if (method.getReturnType().isAssignableFrom(Future.class)) { - ReflectionUtils.rethrowException(ex); - } - else { - // Could not transmit the exception to the caller with default executor - try { - this.exceptionHandler.handleUncaughtException(ex, method, params); - } - catch (Throwable ex2) { - logger.error("Exception handler for async method '" + method.toGenericString() + - "' threw unexpected exception itself", ex2); - } - } - } - /** * This implementation is a no-op for compatibility in Spring 3.1.2. * Subclasses may override to provide support for extracting qualifier information, diff --git a/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AbstractAsyncExecutionAspect.aj b/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AbstractAsyncExecutionAspect.aj index 660b35c1361..2f2455367e1 100644 --- a/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AbstractAsyncExecutionAspect.aj +++ b/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AbstractAsyncExecutionAspect.aj @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -35,6 +35,7 @@ import org.springframework.core.task.AsyncTaskExecutor; * @author Ramnivas Laddad * @author Juergen Hoeller * @author Chris Beams + * @author Stephane Nicoll * @since 3.0.5 */ public abstract aspect AbstractAsyncExecutionAspect extends AsyncExecutionAspectSupport { @@ -42,7 +43,7 @@ public abstract aspect AbstractAsyncExecutionAspect extends AsyncExecutionAspect /** * Create an {@code AnnotationAsyncExecutionAspect} with a {@code null} default * executor, which should instead be set via {@code #aspectOf} and - * {@link #setExecutor(Executor)}. + * {@link #setExecutor(Executor)}. The same applies for {@link #setExceptionHandler} */ public AbstractAsyncExecutionAspect() { super(null); @@ -56,16 +57,21 @@ public abstract aspect AbstractAsyncExecutionAspect extends AsyncExecutionAspect * otherwise. */ Object around() : asyncMethod() { - MethodSignature methodSignature = (MethodSignature) thisJoinPointStaticPart.getSignature(); + final MethodSignature methodSignature = (MethodSignature) thisJoinPointStaticPart.getSignature(); AsyncTaskExecutor executor = determineAsyncExecutor(methodSignature.getMethod()); if (executor == null) { return proceed(); } Callable callable = new Callable() { public Object call() throws Exception { - Object result = proceed(); - if (result instanceof Future) { - return ((Future) result).get(); + try { + Object result = proceed(); + if (result instanceof Future) { + return ((Future) result).get(); + } + } + catch (Throwable ex) { + handleError(ex, methodSignature.getMethod(), thisJoinPoint.getArgs()); } return null; }}; diff --git a/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AspectJAsyncConfiguration.java b/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AspectJAsyncConfiguration.java index 9484ff0c5bb..bb8128d8e14 100644 --- a/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AspectJAsyncConfiguration.java +++ b/spring-aspects/src/main/java/org/springframework/scheduling/aspectj/AspectJAsyncConfiguration.java @@ -29,6 +29,7 @@ import org.springframework.scheduling.config.TaskManagementConfigUtils; * to enable AspectJ-based asynchronous method execution. * * @author Chris Beams + * @author Stephane Nicoll * @since 3.1 * @see EnableAsync * @see org.springframework.scheduling.annotation.AsyncConfigurationSelector @@ -44,6 +45,9 @@ public class AspectJAsyncConfiguration extends AbstractAsyncConfiguration { if (this.executor != null) { asyncAspect.setExecutor(this.executor); } + if (this.exceptionHandler != null) { + asyncAspect.setExceptionHandler(this.exceptionHandler); + } return asyncAspect; } diff --git a/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/AnnotationAsyncExecutionAspectTests.java b/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/AnnotationAsyncExecutionAspectTests.java index d82367ddb07..424fca0959c 100644 --- a/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/AnnotationAsyncExecutionAspectTests.java +++ b/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/AnnotationAsyncExecutionAspectTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.scheduling.aspectj; +import java.lang.reflect.Method; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -23,6 +24,8 @@ import java.util.concurrent.Future; import org.junit.Before; import org.junit.Test; +import org.springframework.aop.interceptor.AsyncUncaughtExceptionHandler; +import org.springframework.aop.interceptor.SimpleAsyncUncaughtExceptionHandler; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.core.task.SimpleAsyncTaskExecutor; @@ -31,6 +34,7 @@ import org.springframework.scheduling.annotation.AsyncResult; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; +import org.springframework.util.ReflectionUtils; import static org.hamcrest.CoreMatchers.*; import static org.hamcrest.Matchers.not; @@ -40,6 +44,7 @@ import static org.junit.Assert.*; * Unit tests for {@link AnnotationAsyncExecutionAspect}. * * @author Ramnivas Laddad + * @author Stephane Nicoll */ public class AnnotationAsyncExecutionAspectTests { @@ -47,6 +52,8 @@ public class AnnotationAsyncExecutionAspectTests { private CountingExecutor executor; + private AsyncUncaughtExceptionHandler defaultExceptionHandler = new SimpleAsyncUncaughtExceptionHandler(); + @Before public void setUp() { Assume.group(TestGroup.PERFORMANCE); @@ -134,6 +141,46 @@ public class AnnotationAsyncExecutionAspectTests { assertThat(e1Thread.get().getName(), startsWith("e1-")); } + @Test + public void exceptionHandlerCalled() { + Method m = ReflectionUtils.findMethod(ClassWithException.class, "failWithVoid"); + TestableAsyncUncaughtExceptionHandler exceptionHandler = new TestableAsyncUncaughtExceptionHandler(); + AnnotationAsyncExecutionAspect.aspectOf().setExceptionHandler(exceptionHandler); + try { + assertFalse("Handler should not have been called", exceptionHandler.isCalled()); + ClassWithException obj = new ClassWithException(); + obj.failWithVoid(); + exceptionHandler.await(3000); + exceptionHandler.assertCalledWith(m, UnsupportedOperationException.class); + } + finally { + AnnotationAsyncExecutionAspect.aspectOf().setExceptionHandler(defaultExceptionHandler); + } + } + + @Test + public void exceptionHandlerNeverThrowsUnexpectedException() { + Method m = ReflectionUtils.findMethod(ClassWithException.class, "failWithVoid"); + TestableAsyncUncaughtExceptionHandler exceptionHandler = new TestableAsyncUncaughtExceptionHandler(true); + AnnotationAsyncExecutionAspect.aspectOf().setExceptionHandler(exceptionHandler); + try { + assertFalse("Handler should not have been called", exceptionHandler.isCalled()); + ClassWithException obj = new ClassWithException(); + try { + obj.failWithVoid(); + exceptionHandler.await(3000); + exceptionHandler.assertCalledWith(m, UnsupportedOperationException.class); + } + catch (Exception ex) { + fail("No unexpected exception should have been received but got " + ex.getMessage()); + } + } + finally { + AnnotationAsyncExecutionAspect.aspectOf().setExceptionHandler(defaultExceptionHandler); + + } + } + @SuppressWarnings("serial") private static class CountingExecutor extends SimpleAsyncTaskExecutor { @@ -224,4 +271,12 @@ public class AnnotationAsyncExecutionAspectTests { return new AsyncResult(Thread.currentThread()); } } + + static class ClassWithException { + + @Async + public void failWithVoid() { + throw new UnsupportedOperationException("failWithVoid"); + } + } } diff --git a/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/TestableAsyncUncaughtExceptionHandler.java b/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/TestableAsyncUncaughtExceptionHandler.java new file mode 100644 index 00000000000..ecd1bc62fba --- /dev/null +++ b/spring-aspects/src/test/java/org/springframework/scheduling/aspectj/TestableAsyncUncaughtExceptionHandler.java @@ -0,0 +1,87 @@ +/* + * Copyright 2002-2014 the original author or authors. + * + * Licensed 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.springframework.scheduling.aspectj; + +import java.lang.reflect.Method; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import org.springframework.aop.interceptor.AsyncUncaughtExceptionHandler; + +import static org.junit.Assert.*; + +/** + * A {@link AsyncUncaughtExceptionHandler} implementation used for testing purposes. + * + * @author Stephane Nicoll + */ +class TestableAsyncUncaughtExceptionHandler + implements AsyncUncaughtExceptionHandler { + + private final CountDownLatch latch = new CountDownLatch(1); + + private UncaughtExceptionDescriptor descriptor; + + private final boolean throwUnexpectedException; + + TestableAsyncUncaughtExceptionHandler() { + this(false); + } + + TestableAsyncUncaughtExceptionHandler(boolean throwUnexpectedException) { + this.throwUnexpectedException = throwUnexpectedException; + } + + @Override + public void handleUncaughtException(Throwable ex, Method method, Object... params) { + descriptor = new UncaughtExceptionDescriptor(ex, method); + this.latch.countDown(); + if (throwUnexpectedException) { + throw new IllegalStateException("Test exception"); + } + } + + public boolean isCalled() { + return descriptor != null; + } + + public void assertCalledWith(Method expectedMethod, Class expectedExceptionType) { + assertNotNull("Handler not called", descriptor); + assertEquals("Wrong exception type", expectedExceptionType, descriptor.ex.getClass()); + assertEquals("Wrong method", expectedMethod, descriptor.method); + } + + public void await(long timeout) { + try { + this.latch.await(timeout, TimeUnit.MILLISECONDS); + } + catch (Exception e) { + Thread.currentThread().interrupt(); + } + } + + private static class UncaughtExceptionDescriptor { + private final Throwable ex; + + private final Method method; + + private UncaughtExceptionDescriptor(Throwable ex, Method method) { + this.ex = ex; + this.method = method; + } + } +} diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/AnnotationAsyncExecutionInterceptor.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/AnnotationAsyncExecutionInterceptor.java index 8519711c58b..af36a8465ce 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/AnnotationAsyncExecutionInterceptor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/AnnotationAsyncExecutionInterceptor.java @@ -46,7 +46,7 @@ public class AnnotationAsyncExecutionInterceptor extends AsyncExecutionIntercept * executor has been qualified at the method level using {@link Async#value()} */ public AnnotationAsyncExecutionInterceptor(Executor defaultExecutor) { - this(defaultExecutor, new SimpleAsyncUncaughtExceptionHandler()); + super(defaultExecutor); } /**