diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncExecutionChainRunnable.java b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncExecutionChainRunnable.java index 5b48f9279b..9a326f0e1f 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncExecutionChainRunnable.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncExecutionChainRunnable.java @@ -35,7 +35,7 @@ import org.springframework.util.Assert; */ public class AsyncExecutionChainRunnable implements Runnable { - private static final Log logger = LogFactory.getLog(AsyncWebRequest.class); + private static final Log logger = LogFactory.getLog(AsyncExecutionChainRunnable.class); private final AsyncWebRequest asyncWebRequest; @@ -62,8 +62,7 @@ public class AsyncExecutionChainRunnable implements Runnable { */ public void run() { try { - logger.debug("Starting async execution chain"); - callable.call(); + this.callable.call(); } catch (StaleAsyncWebRequestException ex) { logger.trace("Could not complete async request", ex); @@ -73,8 +72,8 @@ public class AsyncExecutionChainRunnable implements Runnable { this.asyncWebRequest.sendError(HttpStatus.INTERNAL_SERVER_ERROR, ex.getMessage()); } finally { - logger.debug("Exiting async execution chain"); - asyncWebRequest.complete(); + logger.debug("Completing async request processing"); + this.asyncWebRequest.complete(); } } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java index 8ed1a4ec6e..c897281209 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/AsyncWebRequest.java @@ -21,9 +21,7 @@ import org.springframework.web.context.request.NativeWebRequest; /** - * Extends {@link NativeWebRequest} with methods for starting, completing, and - * configuring async request processing. Abstract underlying mechanisms such as - * the Servlet 3.0 AsyncContext. + * Extend {@link NativeWebRequest} with methods for async request processing. * * @author Rossen Stoyanchev * @since 3.2 @@ -31,44 +29,44 @@ import org.springframework.web.context.request.NativeWebRequest; public interface AsyncWebRequest extends NativeWebRequest { /** - * Set the timeout for asynchronous request processing. When the timeout - * begins depends on the underlying technology. With the Servlet 3 async - * support the timeout begins after the main processing thread has exited - * and has been returned to the container pool. + * Set the timeout for asynchronous request processing in milliseconds. + * In Servlet 3 async request processing, the timeout begins when the + * main processing thread has exited. */ void setTimeout(Long timeout); /** - * Marks the start of async request processing for example. Ensures the - * request remains open to be completed in a separate thread. + * Mark the start of async request processing for example ensuring the + * request remains open in order to be completed in a separate thread. + * @throws IllegalStateException if async processing has started, if it is + * not supported, or if it has completed. */ void startAsync(); /** - * Return {@code true} if async processing has started following a call to - * {@link #startAsync()} and before it has completed. + * Whether async processing is in progress and has not yet completed. */ boolean isAsyncStarted(); /** - * Complete async request processing finalizing the underlying request. + * Complete async request processing making a best effort but without any + * effect if async request processing has already completed for any reason + * including a timeout. */ void complete(); /** - * Send an error to the client. - */ - void sendError(HttpStatus status, String message); - - /** - * Return {@code true} if async processing completed either normally or for - * any other reason such as a timeout or an error. Note that a timeout or a - * (client) error may occur in a separate thread while async processing is - * still in progress in its own thread. Hence the underlying async request - * may become stale and this method may return {@code true} even if - * {@link #complete()} was never actually called. - * @see StaleAsyncWebRequestException + * Whether async processing has completed either normally via calls to + * {@link #complete()} or for other reasons such as a timeout likely + * detected in a separate thread during async request processing. */ boolean isAsyncCompleted(); + /** + * Send an error to the client making a best effort to do so but without any + * effect if async request processing has already completed, for example due + * to a timeout. + */ + void sendError(HttpStatus status, String message); + } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/StaleAsyncRequestCheckingCallable.java b/spring-web/src/main/java/org/springframework/web/context/request/async/StaleAsyncRequestCheckingCallable.java index 0126a5de26..ee229349bd 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/StaleAsyncRequestCheckingCallable.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/StaleAsyncRequestCheckingCallable.java @@ -42,7 +42,7 @@ public class StaleAsyncRequestCheckingCallable extends AbstractDelegatingCallabl Object result = getNextCallable().call(); if (this.asyncWebRequest.isAsyncCompleted()) { throw new StaleAsyncWebRequestException( - "Async request no longer available due to a timed out or a (client) error"); + "Async request no longer available due to a timeout or a (client) error"); } return result; } diff --git a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java index 76d9f66a66..0c92940d76 100644 --- a/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java +++ b/spring-web/src/main/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequest.java @@ -32,10 +32,10 @@ import org.springframework.web.context.request.ServletWebRequest; /** * A Servlet 3.0 implementation of {@link AsyncWebRequest}. * - *

The servlet processing an async request as well as all filters involved - * must async support enabled. This can be done in Java using the Servlet API - * or by adding an {@code true} element to - * servlet and filter declarations in web.xml + *

The servlet and all filters involved in an async request must have async + * support enabled using the Servlet API or by adding an + * {@code true} element to servlet and filter + * declarations in web.xml * * @author Rossen Stoyanchev * @since 3.2 @@ -57,7 +57,6 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } public boolean isAsyncStarted() { - assertNotStale(); return ((this.asyncContext != null) && getRequest().isAsyncStarted()); } @@ -81,12 +80,17 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements } public void complete() { - assertNotStale(); if (!isAsyncCompleted()) { this.asyncContext.complete(); + completeInternal(); } } + private void completeInternal() { + this.asyncContext = null; + this.asyncCompleted.set(true); + } + public void sendError(HttpStatus status, String message) { try { if (!isAsyncCompleted()) { @@ -107,18 +111,19 @@ public class StandardServletAsyncWebRequest extends ServletWebRequest implements // --------------------------------------------------------------------- public void onTimeout(AsyncEvent event) throws IOException { - this.asyncCompleted.set(true); + completeInternal(); + getResponse().sendError(HttpStatus.SERVICE_UNAVAILABLE.value()); } public void onError(AsyncEvent event) throws IOException { - this.asyncCompleted.set(true); + completeInternal(); } public void onStartAsync(AsyncEvent event) throws IOException { } public void onComplete(AsyncEvent event) throws IOException { - this.asyncCompleted.set(true); + completeInternal(); } } diff --git a/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java b/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java index bc10bedd0c..f7e7594218 100644 --- a/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java +++ b/spring-web/src/test/java/org/springframework/web/context/request/async/StandardServletAsyncWebRequestTests.java @@ -16,11 +16,15 @@ package org.springframework.web.context.request.async; + import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; import static org.easymock.EasyMock.reset; import static org.easymock.EasyMock.verify; +import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -57,7 +61,9 @@ public class StandardServletAsyncWebRequestTests { @Test public void isAsyncStarted() throws Exception { - assertEquals(false, this.asyncRequest.isAsyncStarted()); + replay(this.request); + assertEquals("Should be \"false\" before startAsync()", false, this.asyncRequest.isAsyncStarted()); + verify(this.request); startAsync(); @@ -65,25 +71,19 @@ public class StandardServletAsyncWebRequestTests { expect(this.request.isAsyncStarted()).andReturn(true); replay(this.request); - assertTrue(this.asyncRequest.isAsyncStarted()); - } + assertTrue("Should be \"true\" true startAsync()", this.asyncRequest.isAsyncStarted()); + verify(this.request); - @Test - public void isAsyncStarted_stale() throws Exception { this.asyncRequest.onComplete(new AsyncEvent(null)); - try { - this.asyncRequest.isAsyncStarted(); - fail("expected exception"); - } - catch (IllegalStateException ex) { - assertStaleRequestMessage(ex); - } + + assertFalse("Should be \"false\" after complete()", this.asyncRequest.isAsyncStarted()); } @Test public void startAsync() throws Exception { AsyncContext asyncContext = EasyMock.createMock(AsyncContext.class); + reset(this.request); expect(this.request.isAsyncSupported()).andReturn(true); expect(this.request.startAsync(this.request, this.response)).andStubReturn(asyncContext); replay(this.request); @@ -97,6 +97,19 @@ public class StandardServletAsyncWebRequestTests { verify(this.request); } + @Test + public void startAsync_notSupported() throws Exception { + expect(this.request.isAsyncSupported()).andReturn(false); + replay(this.request); + try { + this.asyncRequest.startAsync(); + fail("expected exception"); + } + catch (IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("Async support must be enabled")); + } + } + @Test public void startAsync_alreadyStarted() throws Exception { startAsync(); @@ -128,20 +141,17 @@ public class StandardServletAsyncWebRequestTests { fail("expected exception"); } catch (IllegalStateException ex) { - assertStaleRequestMessage(ex); + assertEquals("Cannot use async request after completion", ex.getMessage()); } } @Test public void complete_stale() throws Exception { this.asyncRequest.onComplete(new AsyncEvent(null)); - try { - this.asyncRequest.complete(); - fail("expected exception"); - } - catch (IllegalStateException ex) { - assertStaleRequestMessage(ex); - } + this.asyncRequest.complete(); + + assertFalse(this.asyncRequest.isAsyncStarted()); + assertTrue(this.asyncRequest.isAsyncCompleted()); } @Test @@ -151,15 +161,10 @@ public class StandardServletAsyncWebRequestTests { } @Test - public void sendError_requestAlreadyCompleted() throws Exception { + public void sendError_stale() throws Exception { this.asyncRequest.onComplete(new AsyncEvent(null)); this.asyncRequest.sendError(HttpStatus.INTERNAL_SERVER_ERROR, "error"); assertEquals(200, this.response.getStatus()); } - - private void assertStaleRequestMessage(IllegalStateException ex) { - assertEquals("Cannot use async request after completion", ex.getMessage()); - } - } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java index 25abdccb91..919b296ab6 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java @@ -398,10 +398,10 @@ public class RequestMappingHandlerAdapter extends AbstractHandlerMethodAdapter i } /** - * Set the timeout for asynchronous request processing. When the timeout - * begins depends on the underlying async technology. With the Servlet 3 - * async support the timeout begins after the main processing thread has - * exited and has been returned to the container pool. + * Set the timeout for asynchronous request processing in milliseconds. + * When the timeout begins depends on the underlying async technology. + * With the Servlet 3 async support the timeout begins after the main + * processing thread has exited and has been returned to the container pool. *

If a value is not provided, the default timeout of the underlying * async technology is used (10 seconds on Tomcat with Servlet 3 async). */ diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java index d2b86bc8d1..69d672acf2 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ServletInvocableHandlerMethod.java @@ -126,6 +126,7 @@ public class ServletInvocableHandlerMethod extends InvocableHandlerMethod { return new AbstractDelegatingCallable() { public Object call() throws Exception { + mavContainer.setRequestHandled(false); new CallableHandlerMethod(getNextCallable()).invokeAndHandle(webRequest, mavContainer, providedArgs); return null; }