From ece481545987ebcd13552b5db1b56be929a9ae78 Mon Sep 17 00:00:00 2001 From: Violeta Georgieva Date: Tue, 4 Apr 2017 21:36:29 +0300 Subject: [PATCH] Ensure ISE will not be thrown when the client disconnects - ServletServerHttpResponse.ResponseAsyncListener#onError/onTimeout must complete the async operation - ServletHttpHandlerAdapter.HandlerResultSubscriber#onComplete must check that the async operation is not completed Issue: SPR-15412 --- .../AbstractListenerReadPublisher.java | 5 ++-- .../reactive/ServletHttpHandlerAdapter.java | 28 +++++++++++++++---- .../reactive/ServletServerHttpResponse.java | 2 ++ 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java index fe4f0a6f56..d048891ae8 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/AbstractListenerReadPublisher.java @@ -49,7 +49,6 @@ public abstract class AbstractListenerReadPublisher implements Publisher { private final AtomicReference state = new AtomicReference<>(State.UNSUBSCRIBED); - @SuppressWarnings("unused") private volatile long demand; @SuppressWarnings("rawtypes") @@ -96,8 +95,8 @@ public abstract class AbstractListenerReadPublisher implements Publisher { * Listeners can call this to notify when a read error has occurred. */ public final void onError(Throwable t) { - if (this.logger.isErrorEnabled()) { - this.logger.error(this.state + " onError: " + t, t); + if (this.logger.isTraceEnabled()) { + this.logger.trace(this.state + " onError: " + t); } this.state.get().onError(this, t); } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java index df777d849e..169664f233 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletHttpHandlerAdapter.java @@ -166,16 +166,32 @@ public class ServletHttpHandlerAdapter implements Servlet { @Override public void onError(Throwable ex) { - logger.error("Could not complete request", ex); - HttpServletResponse response = (HttpServletResponse) this.asyncContext.getResponse(); - response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); - this.asyncContext.complete(); + ServletRequest request = getRequest(); + if (request != null && request.isAsyncStarted()) { + logger.error("Could not complete request", ex); + HttpServletResponse response = (HttpServletResponse) this.asyncContext.getResponse(); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + this.asyncContext.complete(); + } } @Override public void onComplete() { - logger.debug("Successfully completed request"); - this.asyncContext.complete(); + ServletRequest request = getRequest(); + if (request != null && request.isAsyncStarted()) { + logger.debug("Successfully completed request"); + this.asyncContext.complete(); + } + } + + private ServletRequest getRequest() { + ServletRequest request = null; + try { + request = this.asyncContext.getRequest(); + } catch (IllegalStateException ignore) { + // AsyncContext has been recycled and should not be used + } + return request; } } diff --git a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java index 63854ef3dc..e248a0eb81 100644 --- a/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java +++ b/spring-web/src/main/java/org/springframework/http/server/reactive/ServletServerHttpResponse.java @@ -180,11 +180,13 @@ public class ServletServerHttpResponse extends AbstractListenerServerHttpRespons Throwable ex = event.getThrowable(); ex = (ex != null ? ex : new IllegalStateException("Async operation timeout.")); handleError(ex); + event.getAsyncContext().complete(); } @Override public void onError(AsyncEvent event) { handleError(event.getThrowable()); + event.getAsyncContext().complete(); } void handleError(Throwable ex) {