Commit 5a0a2e6b authored by Guus der Kinderen's avatar Guus der Kinderen

OF-885: Do not use async writing in error handling.

When the invocation of the #onTimeout method of a AsyncListener does
not result in a 'complete()', then the servlet container falls back to
default processing. This causes an async response that was started in
that method to fail.
parent d27e8a9e
...@@ -272,7 +272,7 @@ public class HttpBindServlet extends HttpServlet { ...@@ -272,7 +272,7 @@ public class HttpBindServlet extends HttpServlet {
return reader; return reader;
} }
public static void respond(HttpSession session, AsyncContext context, String content) throws IOException public static void respond(HttpSession session, AsyncContext context, String content, boolean async) throws IOException
{ {
final HttpServletResponse response = ((HttpServletResponse) context.getResponse()); final HttpServletResponse response = ((HttpServletResponse) context.getResponse());
final HttpServletRequest request = ((HttpServletRequest) context.getRequest()); final HttpServletRequest request = ((HttpServletRequest) context.getRequest());
...@@ -292,11 +292,17 @@ public class HttpBindServlet extends HttpServlet { ...@@ -292,11 +292,17 @@ public class HttpBindServlet extends HttpServlet {
} }
if (JiveGlobals.getBooleanProperty("log.httpbind.enabled", false)) { if (JiveGlobals.getBooleanProperty("log.httpbind.enabled", false)) {
System.out.println(new Date()+": HTTP SENT(" + session.getStreamID().getID() + "): " + content); System.out.println(new Date() + ": HTTP SENT(" + session.getStreamID().getID() + "): " + content);
} }
final byte[] byteContent = content.getBytes("UTF-8"); final byte[] byteContent = content.getBytes("UTF-8");
response.getOutputStream().setWriteListener( new WriteListenerImpl(context, byteContent) ); if (async) {
response.getOutputStream().setWriteListener(new WriteListenerImpl(context, byteContent));
} else {
context.getResponse().getOutputStream().write(byteContent);
context.getResponse().getOutputStream().flush();
context.complete();
}
} }
private void sendError(HttpSession session, AsyncContext context, BoshBindingError bindingError) private void sendError(HttpSession session, AsyncContext context, BoshBindingError bindingError)
...@@ -309,7 +315,7 @@ public class HttpBindServlet extends HttpServlet { ...@@ -309,7 +315,7 @@ public class HttpBindServlet extends HttpServlet {
if ((session.getMajorVersion() == 1 && session.getMinorVersion() >= 6) || session.getMajorVersion() > 1) if ((session.getMajorVersion() == 1 && session.getMinorVersion() >= 6) || session.getMajorVersion() > 1)
{ {
final String errorBody = createErrorBody(bindingError.getErrorType().getType(), bindingError.getCondition()); final String errorBody = createErrorBody(bindingError.getErrorType().getType(), bindingError.getCondition());
respond(session, context, errorBody); respond(session, context, errorBody, true);
} else { } else {
sendLegacyError(context, bindingError); sendLegacyError(context, bindingError);
} }
......
...@@ -72,7 +72,7 @@ public class HttpConnection { ...@@ -72,7 +72,7 @@ public class HttpConnection {
} }
try { try {
deliverBody(null); deliverBody(null, true);
} }
catch (HttpConnectionClosedException e) { catch (HttpConnectionClosedException e) {
Log.warn("Unexpected exception occurred while trying to close an HttpException.", e); Log.warn("Unexpected exception occurred while trying to close an HttpException.", e);
...@@ -106,11 +106,12 @@ public class HttpConnection { ...@@ -106,11 +106,12 @@ public class HttpConnection {
* sent an empty body. * sent an empty body.
* *
* @param body the XMPP content to be forwarded to the client inside of a body tag. * @param body the XMPP content to be forwarded to the client inside of a body tag.
* @param async when false, this method blocks until the data has been delivered to the client.
* *
* @throws HttpConnectionClosedException when this connection to the client has already received * @throws HttpConnectionClosedException when this connection to the client has already received
* a deliverable to forward to the client * a deliverable to forward to the client
*/ */
public void deliverBody(String body) throws HttpConnectionClosedException, IOException { public void deliverBody(String body, boolean async) throws HttpConnectionClosedException, IOException {
// We only want to use this function once so we will close it when the body is delivered. // We only want to use this function once so we will close it when the body is delivered.
synchronized (this) { synchronized (this) {
if (isClosed) { if (isClosed) {
...@@ -123,7 +124,7 @@ public class HttpConnection { ...@@ -123,7 +124,7 @@ public class HttpConnection {
if (body == null) { if (body == null) {
body = HttpBindServlet.createEmptyBody(false); body = HttpBindServlet.createEmptyBody(false);
} }
HttpBindServlet.respond(this.getSession(), this.context, body); HttpBindServlet.respond(this.getSession(), this.context, body, async);
} }
/** /**
......
...@@ -611,17 +611,17 @@ public class HttpSession extends LocalClientSession { ...@@ -611,17 +611,17 @@ public class HttpSession extends LocalClientSession {
int pauseDuration = HttpBindServlet.getIntAttribute(rootNode.attributeValue("pause"), -1); int pauseDuration = HttpBindServlet.getIntAttribute(rootNode.attributeValue("pause"), -1);
if ("terminate".equals(type)) { if ("terminate".equals(type)) {
connection.deliverBody(createEmptyBody(true)); connection.deliverBody(createEmptyBody(true), true);
close(); close();
lastRequestID = connection.getRequestId(); lastRequestID = connection.getRequestId();
} }
else if ("true".equals(restartStream) && rootNode.elements().size() == 0) { else if ("true".equals(restartStream) && rootNode.elements().size() == 0) {
connection.deliverBody(createSessionRestartResponse()); connection.deliverBody(createSessionRestartResponse(), true);
lastRequestID = connection.getRequestId(); lastRequestID = connection.getRequestId();
} }
else if (pauseDuration > 0 && pauseDuration <= getMaxPause()) { else if (pauseDuration > 0 && pauseDuration <= getMaxPause()) {
pause(pauseDuration); pause(pauseDuration);
connection.deliverBody(createEmptyBody(false)); connection.deliverBody(createEmptyBody(false), true);
lastRequestID = connection.getRequestId(); lastRequestID = connection.getRequestId();
setLastResponseEmpty(true); setLastResponseEmpty(true);
} }
...@@ -700,14 +700,18 @@ public class HttpSession extends LocalClientSession { ...@@ -700,14 +700,18 @@ public class HttpSession extends LocalClientSession {
context.addListener(new AsyncListener() { context.addListener(new AsyncListener() {
@Override @Override
public void onComplete(AsyncEvent asyncEvent) throws IOException { public void onComplete(AsyncEvent asyncEvent) throws IOException {
Log.debug("complete event " + asyncEvent);
connectionQueue.remove(connection); connectionQueue.remove(connection);
fireConnectionClosed(connection); fireConnectionClosed(connection);
} }
@Override @Override
public void onTimeout(AsyncEvent asyncEvent) throws IOException { public void onTimeout(AsyncEvent asyncEvent) throws IOException {
Log.debug("timeout event " + asyncEvent);
try { try {
connection.deliverBody(createEmptyBody()); // If onTimeout does not result in a complete(), the container falls back to default behavior.
// This is why this body is to be delivered in a non-async fashion.
connection.deliverBody(createEmptyBody(), false);
setLastResponseEmpty(true); setLastResponseEmpty(true);
// This connection timed out we need to increment the request count // This connection timed out we need to increment the request count
...@@ -725,6 +729,7 @@ public class HttpSession extends LocalClientSession { ...@@ -725,6 +729,7 @@ public class HttpSession extends LocalClientSession {
@Override @Override
public void onError(AsyncEvent asyncEvent) throws IOException { public void onError(AsyncEvent asyncEvent) throws IOException {
Log.debug("error event " + asyncEvent);
Log.warn("Unhandled AsyncListener error: " + asyncEvent.getThrowable()); Log.warn("Unhandled AsyncListener error: " + asyncEvent.getThrowable());
} }
...@@ -739,7 +744,7 @@ public class HttpSession extends LocalClientSession { ...@@ -739,7 +744,7 @@ public class HttpSession extends LocalClientSession {
throw new HttpBindException("Unexpected RID error.", throw new HttpBindException("Unexpected RID error.",
BoshBindingError.itemNotFound); BoshBindingError.itemNotFound);
} }
connection.deliverBody(createDeliverable(deliverable.deliverables)); connection.deliverBody(createDeliverable(deliverable.deliverables), true);
addConnection(connection, isPoll); addConnection(connection, isPoll);
return connection; return connection;
} }
...@@ -805,7 +810,7 @@ public class HttpSession extends LocalClientSession { ...@@ -805,7 +810,7 @@ public class HttpSession extends LocalClientSession {
throw new HttpBindException("Unexpected RID error.", throw new HttpBindException("Unexpected RID error.",
BoshBindingError.itemNotFound); BoshBindingError.itemNotFound);
} }
connection.deliverBody(createDeliverable(deliverable.deliverables)); connection.deliverBody(createDeliverable(deliverable.deliverables), true);
} else { } else {
if(Log.isDebugEnabled()) { if(Log.isDebugEnabled()) {
Log.debug("It's still open - calling close()"); Log.debug("It's still open - calling close()");
...@@ -889,7 +894,7 @@ public class HttpSession extends LocalClientSession { ...@@ -889,7 +894,7 @@ public class HttpSession extends LocalClientSession {
private void deliver(HttpConnection connection, Collection<Deliverable> deliverable) private void deliver(HttpConnection connection, Collection<Deliverable> deliverable)
throws HttpConnectionClosedException, IOException { throws HttpConnectionClosedException, IOException {
connection.deliverBody(createDeliverable(deliverable)); connection.deliverBody(createDeliverable(deliverable), true);
Delivered delivered = new Delivered(deliverable); Delivered delivered = new Delivered(deliverable);
delivered.setRequestID(connection.getRequestId()); delivered.setRequestID(connection.getRequestId());
...@@ -1041,7 +1046,7 @@ public class HttpSession extends LocalClientSession { ...@@ -1041,7 +1046,7 @@ public class HttpSession extends LocalClientSession {
pendingElements.clear(); pendingElements.clear();
} }
} else { } else {
toClose.deliverBody(null); toClose.deliverBody(null, true);
} }
} }
} catch (HttpConnectionClosedException e) { } catch (HttpConnectionClosedException e) {
......
...@@ -202,7 +202,7 @@ public class HttpSessionManager { ...@@ -202,7 +202,7 @@ public class HttpSessionManager {
connection.setSession(session); connection.setSession(session);
try { try {
connection.deliverBody(createSessionCreationResponse(session)); connection.deliverBody(createSessionCreationResponse(session), true);
} }
catch (HttpConnectionClosedException e) { catch (HttpConnectionClosedException e) {
Log.error("Error creating session.", e); Log.error("Error creating session.", e);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment