---
.../java/org/eclipse/jetty/server/HttpChannelState.java | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 73d78e096497..66daa3fc332b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -51,7 +51,7 @@ public class HttpChannelState
private static final long DEFAULT_TIMEOUT = Long.getLong("org.eclipse.jetty.server.HttpChannelState.DEFAULT_TIMEOUT", 30000L);
- /**
+ /*
* The state of the HttpChannel,used to control the overall lifecycle.
*
* IDLE <-----> HANDLING ----> WAITING
@@ -70,7 +70,7 @@ public enum State
UPGRADED // Request upgraded the connection
}
- /**
+ /*
* The state of the request processing lifecycle.
*
* BLOCKING <----> COMPLETING ---> COMPLETED
@@ -100,7 +100,7 @@ private enum RequestState
COMPLETED // Response is completed
}
- /**
+ /*
* The input readiness state, which works together with {@link HttpInput.State}
*/
private enum InputState
@@ -113,7 +113,7 @@ private enum InputState
READY // isReady() was false, onContentAdded has been called
}
- /**
+ /*
* The output committed state, which works together with {@link HttpOutput.State}
*/
private enum OutputState
From cbce985fc55890fbe85417bfa11e95fc8aded5b6 Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Mon, 5 Aug 2019 14:13:43 +1000
Subject: [PATCH 49/57] cleanups and simplifications
Signed-off-by: Greg Wilkins
---
.../org/eclipse/jetty/server/HttpChannel.java | 19 ++-
.../jetty/server/HttpChannelState.java | 111 +++++++-----------
.../jetty/server/handler/ErrorHandler.java | 9 +-
.../jetty/server/AsyncCompletionTest.java | 1 +
4 files changed, 57 insertions(+), 83 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index 1da494fda5f9..a29073aa673b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -346,10 +346,6 @@ public boolean handle()
// break loop without calling unhandle
break loop;
- case NOOP:
- // do nothing other than call unhandle
- break;
-
case DISPATCH:
{
if (!_request.hasMetaData())
@@ -449,8 +445,15 @@ public boolean handle()
String errorPage = (errorHandler instanceof ErrorPageMapper) ? ((ErrorPageMapper)errorHandler).getErrorPage(_request) : null;
Dispatcher errorDispatcher = errorPage != null ? (Dispatcher)context.getRequestDispatcher(errorPage) : null;
- if (errorDispatcher != null)
+ if (errorDispatcher == null)
+ {
+ // Allow ErrorHandler to generate response
+ errorHandler.handle(null, _request, _request, _response);
+ _request.setHandled(true);
+ }
+ else
{
+ // Do the error page dispatch
try
{
_request.setAttribute(ErrorHandler.ERROR_PAGE, errorPage);
@@ -470,12 +473,6 @@ public boolean handle()
_request.setDispatcherType(null);
}
}
- else
- {
- // Allow ErrorHandler to generate response
- errorHandler.handle(null, _request, _request, _response);
- _request.setHandled(true);
- }
}
catch (Throwable x)
{
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 66daa3fc332b..38a6f1889965 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -129,7 +129,6 @@ private enum OutputState
*/
public enum Action
{
- NOOP, // No action
DISPATCH, // handle a normal request dispatch
ASYNC_DISPATCH, // handle an async request dispatch
SEND_ERROR, // Generate an error page or error dispatch
@@ -391,9 +390,6 @@ public Action handling()
LOG.debug("nextAction(true) {} {}", action, toStringLocked());
return action;
- case WAITING:
- case HANDLING:
- case UPGRADED:
default:
throw new IllegalStateException(getStatusStringLocked());
}
@@ -417,14 +413,8 @@ protected Action unhandle()
if (LOG.isDebugEnabled())
LOG.debug("unhandle {}", toStringLocked());
- switch (_state)
- {
- case HANDLING:
- break;
-
- default:
- throw new IllegalStateException(this.getStatusStringLocked());
- }
+ if (_state != State.HANDLING)
+ throw new IllegalStateException(this.getStatusStringLocked());
_initial = false;
@@ -477,12 +467,8 @@ private Action nextAction(boolean handling)
return Action.READ_CALLBACK;
case REGISTER:
case PRODUCING:
- if (!handling)
- {
- _inputState = InputState.REGISTERED;
- return Action.READ_REGISTER;
- }
- break;
+ _inputState = InputState.REGISTERED;
+ return Action.READ_REGISTER;
case IDLE:
case REGISTERED:
break;
@@ -496,9 +482,6 @@ private Action nextAction(boolean handling)
return Action.WRITE_CALLBACK;
}
- if (handling)
- return Action.NOOP;
-
Scheduler scheduler = _channel.getScheduler();
if (scheduler != null && _timeoutMs > 0 && !_event.hasTimeoutTask())
_event.setTimeoutTask(scheduler.schedule(_event, _timeoutMs, TimeUnit.MILLISECONDS));
@@ -929,11 +912,6 @@ public void sendError(int code, String message)
synchronized (this)
{
- if (_outputState != OutputState.OPEN)
- throw new IllegalStateException("Response is " + _outputState);
- response.getHttpOutput().sendErrorClose();
- response.resetContent(); // will throw ISE if committed
-
if (LOG.isDebugEnabled())
LOG.debug("sendError {}", toStringLocked());
@@ -941,30 +919,30 @@ public void sendError(int code, String message)
{
case HANDLING:
case WOKEN:
- case WAITING:
- request.getResponse().setStatus(code);
- // we are allowed to have a body, then produce the error page.
- ContextHandler.Context context = request.getErrorContext();
- if (context != null)
- request.setAttribute(ErrorHandler.ERROR_CONTEXT, context);
- request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());
- request.setAttribute(ERROR_SERVLET_NAME, request.getServletName());
- request.setAttribute(ERROR_STATUS_CODE, code);
- request.setAttribute(ERROR_MESSAGE, message);
-
- _sendError = true;
- if (_event != null)
- {
- Throwable cause = (Throwable)request.getAttribute(ERROR_EXCEPTION);
- if (cause != null)
- _event.addThrowable(cause);
- }
+ case WAITING:
break;
-
default:
- {
throw new IllegalStateException(getStatusStringLocked());
- }
+ }
+ if (_outputState != OutputState.OPEN)
+ throw new IllegalStateException("Response is " + _outputState);
+
+ response.getHttpOutput().sendErrorClose();
+ response.resetContent();
+ request.getResponse().setStatus(code);
+
+ request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext());
+ request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());
+ request.setAttribute(ERROR_SERVLET_NAME, request.getServletName());
+ request.setAttribute(ERROR_STATUS_CODE, code);
+ request.setAttribute(ERROR_MESSAGE, message);
+
+ _sendError = true;
+ if (_event != null)
+ {
+ Throwable cause = (Throwable)request.getAttribute(ERROR_EXCEPTION);
+ if (cause != null)
+ _event.addThrowable(cause);
}
}
}
@@ -997,29 +975,24 @@ protected void completed()
if (LOG.isDebugEnabled())
LOG.debug("completed {}", toStringLocked());
- switch (_requestState)
- {
- case COMPLETING:
- if (_event == null)
- {
- _requestState = RequestState.COMPLETED;
- aListeners = null;
- event = null;
- if (_state == State.WAITING)
- {
- _state = State.WOKEN;
- handle = true;
- }
- }
- else
- {
- aListeners = _asyncListeners;
- event = _event;
- }
- break;
+ if (_requestState != RequestState.COMPLETING)
+ throw new IllegalStateException(this.getStatusStringLocked());
- default:
- throw new IllegalStateException(this.getStatusStringLocked());
+ if (_event == null)
+ {
+ _requestState = RequestState.COMPLETED;
+ aListeners = null;
+ event = null;
+ if (_state == State.WAITING)
+ {
+ _state = State.WOKEN;
+ handle = true;
+ }
+ }
+ else
+ {
+ aListeners = _asyncListeners;
+ event = _event;
}
}
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
index 52549c9d153a..ba446af78c5b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
@@ -61,6 +61,7 @@ public class ErrorHandler extends AbstractHandler
public static final String ERROR_CONTEXT = "org.eclipse.jetty.server.error_context";
boolean _showStacks = true;
+ boolean _disableStacks = false;
boolean _showMessageInTitle = true;
String _cacheControl = "must-revalidate,no-cache,no-store";
@@ -301,10 +302,10 @@ protected void generateAcceptableResponse(Request baseRequest, HttpServletReques
if (LOG.isDebugEnabled())
LOG.warn(e);
baseRequest.getResponse().resetContent();
- if (_showStacks)
+ if (!_disableStacks)
{
LOG.info("Disabling showsStacks for " + this);
- _showStacks = false;
+ _disableStacks = true;
continue;
}
break;
@@ -355,7 +356,7 @@ protected void writeErrorPageBody(HttpServletRequest request, Writer writer, int
String uri = request.getRequestURI();
writeErrorPageMessage(request, writer, code, message, uri);
- if (showStacks)
+ if (showStacks && !_disableStacks)
writeErrorPageStacks(request, writer);
Request.getBaseRequest(request).getHttpChannel().getHttpConfiguration()
@@ -416,6 +417,8 @@ private void writeErrorPlain(HttpServletRequest request, PrintWriter writer, int
while (cause != null)
{
writer.printf("CAUSED BY %s%n", cause);
+ if (_showStacks && !_disableStacks)
+ cause.printStackTrace(writer);
cause = cause.getCause();
}
}
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncCompletionTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncCompletionTest.java
index 142f86726170..a14a6bcd09f4 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncCompletionTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncCompletionTest.java
@@ -184,6 +184,7 @@ public void testAsyncCompletion(Handler handler, int status, String message) thr
// The write should happen but the callback is delayed
HttpTester.Response response = HttpTester.parseResponse(client.getInputStream());
+ assertThat(response, Matchers.notNullValue());
assertThat(response.getStatus(), is(status));
String content = response.getContent();
assertThat(content, containsString(message));
From e73522255aa154b31a1843ad9a21f875c49d0669 Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Mon, 5 Aug 2019 22:14:29 +1000
Subject: [PATCH 50/57] Cleanup from Review
renaming and some TODOs
Signed-off-by: Greg Wilkins
---
.../org/eclipse/jetty/server/HttpChannel.java | 15 +++++++++------
.../eclipse/jetty/server/HttpChannelState.java | 14 +++++++++-----
.../java/org/eclipse/jetty/server/HttpOutput.java | 2 +-
.../java/org/eclipse/jetty/server/Response.java | 3 +--
.../jetty/server/handler/ErrorHandler.java | 4 ++--
5 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index a29073aa673b..0cb118bac92b 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -419,13 +419,13 @@ public boolean handle()
{
// Get ready to send an error response
_request.setHandled(false);
- _response.resetContent();
+ _response.resetContent(); // TODO should we only do this here ???
_response.getHttpOutput().reopen();
// the following is needed as you cannot trust the response code and reason
// as those could have been modified after calling sendError
Integer icode = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
- int code = icode != null ? icode : HttpStatus.INTERNAL_SERVER_ERROR_500;
+ int code = icode != null ? icode : HttpStatus.INTERNAL_SERVER_ERROR_500; // TODO is this necessary still?
_request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, code);
_response.setStatus(code);
@@ -437,7 +437,7 @@ public boolean handle()
errorHandler == null ||
!errorHandler.errorPageForMethod(_request.getMethod()))
{
- sendCompleteResponse();
+ sendResponseAndComplete();
break;
}
@@ -481,19 +481,22 @@ public boolean handle()
Throwable failure = (Throwable)_request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
if (failure == null)
failure = x;
- else
+ else if (x != failure)
failure.addSuppressed(x);
+ // TODO do we really need to reset the code?
Throwable cause = unwrap(failure, BadMessageException.class);
int code = cause instanceof BadMessageException ? ((BadMessageException)cause).getCode() : 500;
_response.setStatus(code);
if (_state.isResponseCommitted())
+ {
abort(x);
+ }
else
{
_response.resetContent();
- sendCompleteResponse();
+ sendResponseAndComplete();
}
}
finally
@@ -656,7 +659,7 @@ protected Throwable unwrap(Throwable failure, Class>... targets)
return null;
}
- public void sendCompleteResponse()
+ public void sendResponseAndComplete()
{
try
{
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 38a6f1889965..16ed9946fd7f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -504,6 +504,7 @@ private Action nextAction(boolean handling)
// so we will do a normal error dispatch
_requestState = RequestState.BLOCKING;
+ /// TODO explain why we don't just call sendError here????
final Request request = _channel.getRequest();
ContextHandler.Context context = _event.getContext();
if (context != null)
@@ -762,6 +763,8 @@ protected void thrownException(Throwable th)
// + If the request is async, then any async listeners are give a chance to handle the exception in their onError handler.
// + If the request is not async, or not handled by any async onError listener, then a normal sendError is done.
+
+ // TODO make this a method?
Runnable sendError = () ->
{
final Request request = _channel.getRequest();
@@ -798,6 +801,9 @@ else if (cause instanceof UnavailableException)
request.setAttribute(ERROR_EXCEPTION, th);
request.setAttribute(ERROR_EXCEPTION_TYPE, th.getClass());
sendError(code, message);
+
+ // Ensure any async lifecycle is ended!
+ _requestState = RequestState.BLOCKING;
};
final AsyncContextEvent asyncEvent;
@@ -836,7 +842,6 @@ else if (cause instanceof UnavailableException)
if (_asyncListeners == null || _asyncListeners.isEmpty())
{
sendError.run();
- _requestState = RequestState.BLOCKING;
return;
}
asyncEvent = _event;
@@ -881,7 +886,6 @@ else if (cause instanceof UnavailableException)
// The listeners did not invoke API methods
// and the container must provide a default error dispatch.
sendError.run();
- _requestState = RequestState.BLOCKING;
return;
case DISPATCH:
@@ -927,9 +931,9 @@ public void sendError(int code, String message)
if (_outputState != OutputState.OPEN)
throw new IllegalStateException("Response is " + _outputState);
- response.getHttpOutput().sendErrorClose();
- response.resetContent();
- request.getResponse().setStatus(code);
+ response.getHttpOutput().closedBySendError();
+ response.resetContent(); // TODO do we need to do this here?
+ response.setStatus(code);
request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext());
request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
index dddd5e0eded9..664dd96412da 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
@@ -264,7 +264,7 @@ private void abort(Throwable failure)
_channel.abort(failure);
}
- public void sendErrorClose()
+ public void closedBySendError()
{
while (true)
{
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
index 0a1ca984ccec..b896a10c074d 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
@@ -407,7 +407,7 @@ public void sendError(int code, String message) throws IOException
case -1:
_channel.abort(new IOException(message));
break;
- case 102:
+ case HttpStatus.PROCESSING_102:
sendProcessing();
break;
default:
@@ -1085,7 +1085,6 @@ public void resetContent()
i.remove();
continue;
default:
- continue;
}
}
}
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
index ba446af78c5b..65eacba85ef5 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
@@ -265,7 +265,7 @@ protected void generateAcceptableResponse(Request baseRequest, HttpServletReques
// We will write it into a byte array buffer so
// we can flush it asynchronously.
- while(true)
+ while (true)
{
try
{
@@ -312,7 +312,7 @@ protected void generateAcceptableResponse(Request baseRequest, HttpServletReques
}
}
- baseRequest.getHttpChannel().sendCompleteResponse();
+ baseRequest.getHttpChannel().sendResponseAndComplete();
}
protected void handleErrorPage(HttpServletRequest request, Writer writer, int code, String message)
From 73ca4774c8bbef9ebd3adffe835d533a3ca2f006 Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Tue, 6 Aug 2019 08:52:23 +1000
Subject: [PATCH 51/57] Cleanup from Review
Checkstyle fixes
Signed-off-by: Greg Wilkins
---
.../org/eclipse/jetty/proxy/AbstractProxyServlet.java | 2 +-
.../org/eclipse/jetty/server/handler/ErrorHandler.java | 9 ++++-----
2 files changed, 5 insertions(+), 6 deletions(-)
diff --git a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java
index c670df04766a..3e6f53410bbe 100644
--- a/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java
+++ b/jetty-proxy/src/main/java/org/eclipse/jetty/proxy/AbstractProxyServlet.java
@@ -684,7 +684,7 @@ protected void sendProxyResponseError(HttpServletRequest clientRequest, HttpServ
{
proxyResponse.sendError(-1);
}
- catch(Exception e2)
+ catch (Exception e2)
{
_log.ignore(e2);
}
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
index 65eacba85ef5..2d05b4fb9c93 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
@@ -380,7 +380,7 @@ protected void writeErrorPageMessage(HttpServletRequest request, Writer writer,
htmlRow(writer, "STATUS", status);
htmlRow(writer, "MESSAGE", message);
htmlRow(writer, "SERVLET", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
- Throwable cause = (Throwable) request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+ Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
while (cause != null)
{
htmlRow(writer, "CAUSED BY", cause);
@@ -413,7 +413,7 @@ private void writeErrorPlain(HttpServletRequest request, PrintWriter writer, int
writer.printf("STATUS: %s%n", code);
writer.printf("MESSAGE: %s%n", message);
writer.printf("SERVLET: %s%n", request.getAttribute(Dispatcher.ERROR_SERVLET_NAME));
- Throwable cause = (Throwable) request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+ Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
while (cause != null)
{
writer.printf("CAUSED BY %s%n", cause);
@@ -431,9 +431,9 @@ private void writeErrorJson(HttpServletRequest request, PrintWriter writer, int
.append(" status: \"").append(Integer.toString(code)).append("\",\n")
.append(" message: ").append(QuotedStringTokenizer.quote(message)).append(",\n");
Object servlet = request.getAttribute(Dispatcher.ERROR_SERVLET_NAME);
- if (servlet !=null)
+ if (servlet != null)
writer.append("servlet: \"").append(servlet.toString()).append("\",\n");
- Throwable cause = (Throwable) request.getAttribute(Dispatcher.ERROR_EXCEPTION);
+ Throwable cause = (Throwable)request.getAttribute(Dispatcher.ERROR_EXCEPTION);
int c = 0;
while (cause != null)
{
@@ -444,7 +444,6 @@ private void writeErrorJson(HttpServletRequest request, PrintWriter writer, int
writer.append("}");
}
-
protected void writeErrorPageStacks(HttpServletRequest request, Writer writer)
throws IOException
{
From 3873cd2015172b270d5ab67cfdc6ede50d42d21f Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Tue, 6 Aug 2019 11:51:04 +1000
Subject: [PATCH 52/57] Cleanup from Review
Code cleanups and simplifications
Signed-off-by: Greg Wilkins
---
.../org/eclipse/jetty/server/HttpChannel.java | 138 ++++++-----------
.../jetty/server/HttpChannelState.java | 146 +++++++-----------
.../jetty/server/HttpConfiguration.java | 3 +
.../jetty/server/handler/ContextHandler.java | 2 +-
.../jetty/server/handler/ErrorHandler.java | 8 +-
5 files changed, 117 insertions(+), 180 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index 0cb118bac92b..fb7763b2a782 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -32,6 +32,7 @@
import java.util.function.Supplier;
import javax.servlet.DispatcherType;
import javax.servlet.RequestDispatcher;
+import javax.servlet.ServletException;
import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.HttpFields;
@@ -353,35 +354,17 @@ public boolean handle()
_request.setHandled(false);
_response.getHttpOutput().reopen();
- try
+ dispatch(DispatcherType.REQUEST, () ->
{
- _request.setDispatcherType(DispatcherType.REQUEST);
- notifyBeforeDispatch(_request);
-
- List customizers = _configuration.getCustomizers();
- if (!customizers.isEmpty())
+ for (HttpConfiguration.Customizer customizer : _configuration.getCustomizers())
{
- for (HttpConfiguration.Customizer customizer : customizers)
- {
- customizer.customize(getConnector(), _configuration, _request);
- if (_request.isHandled())
- break;
- }
+ customizer.customize(getConnector(), _configuration, _request);
+ if (_request.isHandled())
+ return;
}
+ getServer().handle(HttpChannel.this);
+ });
- if (!_request.isHandled())
- getServer().handle(this);
- }
- catch (Throwable x)
- {
- notifyDispatchFailure(_request, x);
- throw x;
- }
- finally
- {
- notifyAfterDispatch(_request);
- _request.setDispatcherType(null);
- }
break;
}
@@ -390,22 +373,7 @@ public boolean handle()
_request.setHandled(false);
_response.getHttpOutput().reopen();
- try
- {
- _request.setDispatcherType(DispatcherType.ASYNC);
- notifyBeforeDispatch(_request);
- getServer().handleAsync(this);
- }
- catch (Throwable x)
- {
- notifyDispatchFailure(_request, x);
- throw x;
- }
- finally
- {
- notifyAfterDispatch(_request);
- _request.setDispatcherType(null);
- }
+ dispatch(DispatcherType.ASYNC,() -> getServer().handleAsync(this));
break;
}
@@ -419,32 +387,27 @@ public boolean handle()
{
// Get ready to send an error response
_request.setHandled(false);
- _response.resetContent(); // TODO should we only do this here ???
+ _response.resetContent();
_response.getHttpOutput().reopen();
// the following is needed as you cannot trust the response code and reason
// as those could have been modified after calling sendError
- Integer icode = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
- int code = icode != null ? icode : HttpStatus.INTERNAL_SERVER_ERROR_500; // TODO is this necessary still?
- _request.setAttribute(RequestDispatcher.ERROR_STATUS_CODE, code);
- _response.setStatus(code);
+ Integer code = (Integer)_request.getAttribute(RequestDispatcher.ERROR_STATUS_CODE);
+ _response.setStatus(code != null ? code : HttpStatus.INTERNAL_SERVER_ERROR_500);
ContextHandler.Context context = (ContextHandler.Context)_request.getAttribute(ErrorHandler.ERROR_CONTEXT);
ErrorHandler errorHandler = ErrorHandler.getErrorHandler(getServer(), context == null ? null : context.getContextHandler());
// If we can't have a body, then create a minimal error response.
- if (HttpStatus.hasNoBody(code) ||
- errorHandler == null ||
- !errorHandler.errorPageForMethod(_request.getMethod()))
+ if (HttpStatus.hasNoBody(_response.getStatus()) || errorHandler == null || !errorHandler.errorPageForMethod(_request.getMethod()))
{
sendResponseAndComplete();
break;
}
- // Look for an error page
+ // Look for an error page dispatcher
String errorPage = (errorHandler instanceof ErrorPageMapper) ? ((ErrorPageMapper)errorHandler).getErrorPage(_request) : null;
Dispatcher errorDispatcher = errorPage != null ? (Dispatcher)context.getRequestDispatcher(errorPage) : null;
-
if (errorDispatcher == null)
{
// Allow ErrorHandler to generate response
@@ -454,45 +417,15 @@ public boolean handle()
else
{
// Do the error page dispatch
- try
- {
- _request.setAttribute(ErrorHandler.ERROR_PAGE, errorPage);
- _request.setDispatcherType(DispatcherType.ERROR);
- notifyBeforeDispatch(_request);
- errorDispatcher.error(_request, _response);
- break;
- }
- catch (Throwable x)
- {
- notifyDispatchFailure(_request, x);
- throw x;
- }
- finally
- {
- notifyAfterDispatch(_request);
- _request.setDispatcherType(null);
- }
+ dispatch(DispatcherType.ERROR,() -> errorDispatcher.error(_request, _response));
}
}
catch (Throwable x)
{
if (LOG.isDebugEnabled())
LOG.debug("Could not perform ERROR dispatch, aborting", x);
- Throwable failure = (Throwable)_request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
- if (failure == null)
- failure = x;
- else if (x != failure)
- failure.addSuppressed(x);
-
- // TODO do we really need to reset the code?
- Throwable cause = unwrap(failure, BadMessageException.class);
- int code = cause instanceof BadMessageException ? ((BadMessageException)cause).getCode() : 500;
- _response.setStatus(code);
-
if (_state.isResponseCommitted())
- {
abort(x);
- }
else
{
_response.resetContent();
@@ -564,6 +497,11 @@ else if (x != failure)
}
}
+ // TODO Currently a blocking/aborting consumeAll is done in the handling of the TERMINATED
+ // TODO Action triggered by the completed callback below. It would be possible to modify the
+ // TODO callback to do a non-blocking consumeAll at this point and only call completed when
+ // TODO that is done.
+
// Set a close callback on the HttpOutput to make it an async callback
_response.getHttpOutput().setClosedCallback(Callback.from(_state::completed));
_response.closeOutput();
@@ -571,14 +509,11 @@ else if (x != failure)
if (_response.getHttpOutput().getClosedCallback() != null)
_response.getHttpOutput().getClosedCallback().succeeded();
- // TODO we could do an asynchronous consumeAll in the callback
break;
}
default:
- {
throw new IllegalStateException(this.toString());
- }
}
}
catch (Throwable failure)
@@ -599,6 +534,26 @@ else if (x != failure)
return !suspended;
}
+ private void dispatch(DispatcherType type, Dispatchable dispatchable) throws IOException, ServletException
+ {
+ try
+ {
+ _request.setDispatcherType(type);
+ notifyBeforeDispatch(_request);
+ dispatchable.dispatch();
+ }
+ catch (Throwable x)
+ {
+ notifyDispatchFailure(_request, x);
+ throw x;
+ }
+ finally
+ {
+ notifyAfterDispatch(_request);
+ _request.setDispatcherType(null);
+ }
+ }
+
/**
* Sends an error 500, performing a special logic to detect whether the request is suspended,
* to avoid concurrent writes from the application.
@@ -635,7 +590,7 @@ else if (noStack != null)
if (isCommitted())
abort(failure);
else
- _state.thrownException(failure);
+ _state.onError(failure);
}
/**
@@ -784,7 +739,7 @@ public void onBadMessage(BadMessageException failure)
{
int status = failure.getCode();
String reason = failure.getReason();
- if (status < 400 || status > 599)
+ if (status < HttpStatus.BAD_REQUEST_400 || status > 599)
failure = new BadMessageException(HttpStatus.BAD_REQUEST_400, reason, failure);
notifyRequestFailure(_request, failure);
@@ -855,7 +810,9 @@ public boolean sendResponse(MetaData.Response info, ByteBuffer content, boolean
// wrap callback to process 100 responses
final int status = info.getStatus();
- final Callback committed = (status < 200 && status >= 100) ? new Send100Callback(callback) : new SendCallback(callback, content, true, complete);
+ final Callback committed = (status < HttpStatus.OK_200 && status >= HttpStatus.CONTINUE_100)
+ ? new Send100Callback(callback)
+ : new SendCallback(callback, content, true, complete);
notifyResponseBegin(_request);
@@ -1109,6 +1066,11 @@ private void notifyEvent2(Function> fun
}
}
+ interface Dispatchable
+ {
+ void dispatch() throws IOException, ServletException;
+ }
+
/**
* Listener for {@link HttpChannel} events.
* HttpChannel will emit events for the various phases it goes through while
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
index 16ed9946fd7f..d75eb7a7e5b2 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelState.java
@@ -499,19 +499,10 @@ private Action nextAction(boolean handling)
case EXPIRING:
if (handling)
throw new IllegalStateException(getStatusStringLocked());
-
- // We must have already called onTimeout and nothing changed,
- // so we will do a normal error dispatch
+ sendError(HttpStatus.INTERNAL_SERVER_ERROR_500, "AsyncContext timeout");
+ // handle sendError immediately
_requestState = RequestState.BLOCKING;
-
- /// TODO explain why we don't just call sendError here????
- final Request request = _channel.getRequest();
- ContextHandler.Context context = _event.getContext();
- if (context != null)
- request.setAttribute(ErrorHandler.ERROR_CONTEXT, context);
- request.setAttribute(ERROR_REQUEST_URI, request.getRequestURI());
- request.setAttribute(ERROR_STATUS_CODE, 500);
- request.setAttribute(ERROR_MESSAGE, "AsyncContext timeout");
+ _sendError = false;
return Action.SEND_ERROR;
case COMPLETE:
@@ -757,55 +748,8 @@ public void asyncError(Throwable failure)
}
}
- protected void thrownException(Throwable th)
+ protected void onError(Throwable th)
{
- // This method is called by HttpChannel.handleException to handle an exception thrown from a dispatch:
- // + If the request is async, then any async listeners are give a chance to handle the exception in their onError handler.
- // + If the request is not async, or not handled by any async onError listener, then a normal sendError is done.
-
-
- // TODO make this a method?
- Runnable sendError = () ->
- {
- final Request request = _channel.getRequest();
-
- // Determine the actual details of the exception
- final int code;
- final String message;
- Throwable cause = _channel.unwrap(th, BadMessageException.class, UnavailableException.class);
- if (cause == null)
- {
- code = HttpStatus.INTERNAL_SERVER_ERROR_500;
- message = th.toString();
- }
- else if (cause instanceof BadMessageException)
- {
- BadMessageException bme = (BadMessageException)cause;
- code = bme.getCode();
- message = bme.getReason();
- }
- else if (cause instanceof UnavailableException)
- {
- message = cause.toString();
- if (((UnavailableException)cause).isPermanent())
- code = HttpStatus.NOT_FOUND_404;
- else
- code = HttpStatus.SERVICE_UNAVAILABLE_503;
- }
- else
- {
- code = HttpStatus.INTERNAL_SERVER_ERROR_500;
- message = null;
- }
-
- request.setAttribute(ERROR_EXCEPTION, th);
- request.setAttribute(ERROR_EXCEPTION_TYPE, th.getClass());
- sendError(code, message);
-
- // Ensure any async lifecycle is ended!
- _requestState = RequestState.BLOCKING;
- };
-
final AsyncContextEvent asyncEvent;
final List asyncListeners;
synchronized (this)
@@ -817,6 +761,7 @@ else if (cause instanceof UnavailableException)
if (_state != State.HANDLING)
throw new IllegalStateException(getStatusStringLocked());
+ // If sendError has already been called, we can only handle one failure at a time!
if (_sendError)
{
LOG.warn("unhandled due to prior sendError", th);
@@ -828,20 +773,15 @@ else if (cause instanceof UnavailableException)
{
case BLOCKING:
// handle the exception with a sendError
- sendError.run();
+ sendError(th);
return;
- case DISPATCH:
- case COMPLETE:
- // Complete or Dispatch have been called, but the original subsequently threw an exception.
- // TODO // GW I think we really should ignore, but will fall through for now.
- // TODO LOG.warn("unhandled due to prior dispatch/complete", th);
- // TODO return;
-
+ case DISPATCH: // Dispatch has already been called but we ignore and handle exception below
+ case COMPLETE: // Complete has already been called but we ignore and handle exception below
case ASYNC:
if (_asyncListeners == null || _asyncListeners.isEmpty())
{
- sendError.run();
+ sendError(th);
return;
}
asyncEvent = _event;
@@ -876,28 +816,57 @@ else if (cause instanceof UnavailableException)
// check the actions of the listeners
synchronized (this)
{
- // if anybody has called sendError then we've handled as much as we can by calling listeners
- if (_sendError)
- return;
-
- switch (_requestState)
- {
- case ASYNC:
- // The listeners did not invoke API methods
- // and the container must provide a default error dispatch.
- sendError.run();
- return;
+ // If we are still async and nobody has called sendError
+ if (_requestState == RequestState.ASYNC && !_sendError)
+ // Then the listeners did not invoke API methods
+ // and the container must provide a default error dispatch.
+ sendError(th);
+ else
+ LOG.warn("unhandled in state " + _requestState, new IllegalStateException(th));
+ }
+ }
- case DISPATCH:
- case COMPLETE:
- // The listeners handled the exception by calling dispatch() or complete().
- return;
+ private void sendError(Throwable th)
+ {
+ // No sync as this is always called with lock held
- default:
- LOG.warn("unhandled in state " + _requestState, new IllegalStateException(th));
- return;
- }
+ // Determine the actual details of the exception
+ final Request request = _channel.getRequest();
+ final int code;
+ final String message;
+ Throwable cause = _channel.unwrap(th, BadMessageException.class, UnavailableException.class);
+ if (cause == null)
+ {
+ code = HttpStatus.INTERNAL_SERVER_ERROR_500;
+ message = th.toString();
}
+ else if (cause instanceof BadMessageException)
+ {
+ BadMessageException bme = (BadMessageException)cause;
+ code = bme.getCode();
+ message = bme.getReason();
+ }
+ else if (cause instanceof UnavailableException)
+ {
+ message = cause.toString();
+ if (((UnavailableException)cause).isPermanent())
+ code = HttpStatus.NOT_FOUND_404;
+ else
+ code = HttpStatus.SERVICE_UNAVAILABLE_503;
+ }
+ else
+ {
+ code = HttpStatus.INTERNAL_SERVER_ERROR_500;
+ message = null;
+ }
+
+ sendError(code, message);
+
+ // No ISE, so good to modify request/state
+ request.setAttribute(ERROR_EXCEPTION, th);
+ request.setAttribute(ERROR_EXCEPTION_TYPE, th.getClass());
+ // Ensure any async lifecycle is ended!
+ _requestState = RequestState.BLOCKING;
}
public void sendError(int code, String message)
@@ -932,7 +901,6 @@ public void sendError(int code, String message)
throw new IllegalStateException("Response is " + _outputState);
response.getHttpOutput().closedBySendError();
- response.resetContent(); // TODO do we need to do this here?
response.setStatus(code);
request.setAttribute(ErrorHandler.ERROR_CONTEXT, request.getErrorContext());
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
index 4de4848708c1..c1b385babe67 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
@@ -19,6 +19,7 @@
package org.eclipse.jetty.server;
import java.io.IOException;
+import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -158,6 +159,8 @@ public void addCustomizer(Customizer customizer)
public List getCustomizers()
{
+ if (_customizers.isEmpty())
+ return Collections.emptyList();
return _customizers;
}
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
index afec04af9d79..80f43b2d3371 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ContextHandler.java
@@ -1103,7 +1103,7 @@ public boolean checkContext(final String target, final Request baseRequest, fina
case UNAVAILABLE:
baseRequest.setHandled(true);
response.sendError(HttpServletResponse.SC_SERVICE_UNAVAILABLE);
- return true;
+ return false;
default:
if ((DispatcherType.REQUEST.equals(dispatch) && baseRequest.isHandled()))
return false;
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
index 2d05b4fb9c93..14700779b34e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ErrorHandler.java
@@ -263,12 +263,15 @@ protected void generateAcceptableResponse(Request baseRequest, HttpServletReques
return;
}
- // We will write it into a byte array buffer so
- // we can flush it asynchronously.
+ // write into the response aggregate buffer and flush it asynchronously.
while (true)
{
try
{
+ // TODO currently the writer used here is of fixed size, so a large
+ // TODO error page may cause a BufferOverflow. In which case we try
+ // TODO again with stacks disabled. If it still overflows, it is
+ // TODO written without a body.
ByteBuffer buffer = baseRequest.getResponse().getHttpOutput().acquireBuffer();
ByteBufferOutputStream out = new ByteBufferOutputStream(buffer);
PrintWriter writer = new PrintWriter(new OutputStreamWriter(out, charset));
@@ -312,6 +315,7 @@ protected void generateAcceptableResponse(Request baseRequest, HttpServletReques
}
}
+ // Do an asynchronous completion.
baseRequest.getHttpChannel().sendResponseAndComplete();
}
From 0d7cd741cae8580f5a422a60d4a38aa840a88a14 Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Wed, 7 Aug 2019 12:03:15 +1000
Subject: [PATCH 53/57] fixed debug
Signed-off-by: Greg Wilkins
---
.../src/main/java/org/eclipse/jetty/server/HttpChannel.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index fb7763b2a782..d2647d3a0d9c 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -716,7 +716,7 @@ public boolean onRequestComplete()
public void onCompleted()
{
if (LOG.isDebugEnabled())
- LOG.debug("COMPLETE for {} written={}", getRequest().getRequestURI(), getBytesWritten());
+ LOG.debug("onCompleted for {} written={}", getRequest().getRequestURI(), getBytesWritten());
if (_requestLog != null)
_requestLog.log(_request, _response);
From aaaa71ef21f9ab0251b969bf90e4c7264f792f53 Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Wed, 7 Aug 2019 16:37:06 +1000
Subject: [PATCH 54/57] Cleanup from Review
Ensure response sent before server shutdown
Signed-off-by: Greg Wilkins
---
.../java/org/eclipse/jetty/server/handler/ShutdownHandler.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ShutdownHandler.java b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ShutdownHandler.java
index 24a1258b61d0..ed5affa51c9e 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ShutdownHandler.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ShutdownHandler.java
@@ -197,8 +197,9 @@ protected void doShutdown(Request baseRequest, HttpServletResponse response) thr
connector.shutdown();
}
- response.sendError(200, "Connectors closed, commencing full shutdown");
baseRequest.setHandled(true);
+ response.setStatus(200);
+ response.flushBuffer();
final Server server = getServer();
new Thread()
From da5eb1ea5e22f9fdd744efe57112c71ea47f2d6f Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Tue, 13 Aug 2019 10:13:14 +1000
Subject: [PATCH 55/57] removed unnecessary optimisation
Signed-off-by: Greg Wilkins
---
.../main/java/org/eclipse/jetty/server/HttpConfiguration.java | 3 ---
1 file changed, 3 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
index e68c1e0f5093..9aa9b62fcc86 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
@@ -19,7 +19,6 @@
package org.eclipse.jetty.server;
import java.io.IOException;
-import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
@@ -160,8 +159,6 @@ public void addCustomizer(Customizer customizer)
public List getCustomizers()
{
- if (_customizers.isEmpty())
- return Collections.emptyList();
return _customizers;
}
From 98f629646604352f962643759721fb20148f999f Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Wed, 21 Aug 2019 12:10:14 +1000
Subject: [PATCH 56/57] fixed duplicate from merge
Signed-off-by: Greg Wilkins
---
.../jetty/websocket/tests/WebSocketConnectionStatsTest.java | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java
index 812a75cc0a77..148720950c40 100644
--- a/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java
+++ b/jetty-websocket/jetty-websocket-tests/src/test/java/org/eclipse/jetty/websocket/tests/WebSocketConnectionStatsTest.java
@@ -123,7 +123,6 @@ long getFrameByteSize(WebSocketFrame frame)
@Disabled("Flaky test see issue #3982")
@Test
- @Disabled // TODO this is a flakey test
public void echoStatsTest() throws Exception
{
URI uri = URI.create("ws://localhost:" + connector.getLocalPort() + "/testPath");
@@ -177,4 +176,4 @@ public void echoStatsTest() throws Exception
assertThat("stats.sendBytes", statistics.getSentBytes(), is(expectedSent));
assertThat("stats.receivedBytes", statistics.getReceivedBytes(), is(expectedReceived));
}
-}
\ No newline at end of file
+}
From c62add8ae7a15e23cc71b69ae41cb9023c9aba1e Mon Sep 17 00:00:00 2001
From: Greg Wilkins
Date: Thu, 22 Aug 2019 18:29:51 +1000
Subject: [PATCH 57/57] Updates from review
Signed-off-by: Greg Wilkins
---
.../org/eclipse/jetty/server/HttpChannel.java | 6 +--
.../org/eclipse/jetty/server/HttpOutput.java | 45 ++++++++++++-------
.../org/eclipse/jetty/server/Response.java | 7 +++
.../jetty/util/thread/QueuedThreadPool.java | 8 +---
4 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
index fbeebae57c7f..31714973255f 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java
@@ -503,11 +503,7 @@ public boolean handle()
// TODO that is done.
// Set a close callback on the HttpOutput to make it an async callback
- _response.getHttpOutput().setClosedCallback(Callback.from(_state::completed));
- _response.closeOutput();
- // ensure the callback actually got called
- if (_response.getHttpOutput().getClosedCallback() != null)
- _response.getHttpOutput().getClosedCallback().succeeded();
+ _response.closeOutput(Callback.from(_state::completed));
break;
}
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
index 664dd96412da..655e8cd5dbdb 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
@@ -18,6 +18,7 @@
package org.eclipse.jetty.server;
+import java.io.Closeable;
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
@@ -284,27 +285,36 @@ public void closedBySendError()
}
}
- /**
- * Make {@link #close()} am asynchronous method
- * @param closeCallback The callback to use when close() is called.
- */
- public void setClosedCallback(Callback closeCallback)
+ public void close(Closeable wrapper, Callback callback)
{
- _closeCallback = closeCallback;
- }
-
- public Callback getClosedCallback()
- {
- return _closeCallback;
+ _closeCallback = callback;
+ try
+ {
+ if (wrapper != null)
+ wrapper.close();
+ if (!isClosed())
+ close();
+ }
+ catch (Throwable th)
+ {
+ closed();
+ if (_closeCallback == null)
+ LOG.ignore(th);
+ else
+ callback.failed(th);
+ }
+ finally
+ {
+ if (_closeCallback != null)
+ callback.succeeded();
+ _closeCallback = null;
+ }
}
@Override
public void close()
{
- Callback closeCallback = _closeCallback;
- _closeCallback = null;
- if (closeCallback == null)
- closeCallback = BLOCKING_CLOSE_CALLBACK;
+ Callback closeCallback = _closeCallback == null ? BLOCKING_CLOSE_CALLBACK : _closeCallback;
while (true)
{
@@ -314,6 +324,7 @@ public void close()
case CLOSING:
case CLOSED:
{
+ _closeCallback = null;
closeCallback.succeeded();
return;
}
@@ -342,6 +353,7 @@ public void close()
LOG.warn(ex.toString());
LOG.debug(ex);
abort(ex);
+ _closeCallback = null;
closeCallback.failed(ex);
return;
}
@@ -359,16 +371,19 @@ public void close()
{
// Do a blocking close
write(content, !_channel.getResponse().isIncluding());
+ _closeCallback = null;
closeCallback.succeeded();
}
else
{
+ _closeCallback = null;
write(content, !_channel.getResponse().isIncluding(), closeCallback);
}
}
catch (IOException x)
{
LOG.ignore(x); // Ignore it, it's been already logged in write().
+ _closeCallback = null;
closeCallback.failed(x);
}
return;
diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
index f58d7e77ab00..04870c509492 100644
--- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
+++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java
@@ -18,6 +18,7 @@
package org.eclipse.jetty.server;
+import java.io.Closeable;
import java.io.IOException;
import java.io.PrintWriter;
import java.nio.channels.IllegalSelectorException;
@@ -57,6 +58,7 @@
import org.eclipse.jetty.http.PreEncodedHttpField;
import org.eclipse.jetty.io.RuntimeIOException;
import org.eclipse.jetty.server.session.SessionHandler;
+import org.eclipse.jetty.util.Callback;
import org.eclipse.jetty.util.StringUtil;
import org.eclipse.jetty.util.URIUtil;
import org.eclipse.jetty.util.log.Log;
@@ -801,6 +803,11 @@ public void closeOutput() throws IOException
_out.close();
}
+ public void closeOutput(Callback callback)
+ {
+ _out.close((_outputType == OutputType.WRITER) ? _writer : _out, callback);
+ }
+
public long getLongContentLength()
{
return _contentLength;
diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java
index 3f5addd27bba..41f02b80e8bf 100644
--- a/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java
+++ b/jetty-util/src/main/java/org/eclipse/jetty/util/thread/QueuedThreadPool.java
@@ -171,10 +171,11 @@ protected void doStop() throws Exception
if (LOG.isDebugEnabled())
LOG.debug("Stopping {}", this);
+ super.doStop();
+
removeBean(_tryExecutor);
_tryExecutor = TryExecutor.NO_TRY;
- super.doStop();
// Signal the Runner threads that we are stopping
int threads = _counts.getAndSetHi(Integer.MIN_VALUE);
@@ -184,11 +185,6 @@ protected void doStop() throws Exception
BlockingQueue jobs = getQueue();
if (timeout > 0)
{
- // Consume any reserved threads
- while (tryExecute(NOOP))
- {
- }
-
// Fill the job queue with noop jobs to wakeup idle threads.
for (int i = 0; i < threads; ++i)
{