diff options
Diffstat (limited to 'jdisc_http_service/src/main/java/com/yahoo/jdisc')
4 files changed, 20 insertions, 10 deletions
diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java index e9aba0cb6c9..c16ac589332 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestDispatch.java @@ -101,6 +101,13 @@ class HttpRequestDispatch { HttpRequestDispatch parent = this; //used to avoid binding uninitialized variables completeRequestCallback = (result, error) -> { + boolean alreadyCalled = completeRequestCalled.getAndSet(true); + if (alreadyCalled) { + AssertionError e = new AssertionError("completeRequest called more than once"); + log.log(Level.WARNING, "Assertion failed.", e); + throw e; + } + boolean reportedError = false; if (error != null) { @@ -113,14 +120,6 @@ class HttpRequestDispatch { parent.metricReporter.successfulResponse(); } - - boolean alreadyCalled = completeRequestCalled.getAndSet(true); - if (alreadyCalled) { - AssertionError e = new AssertionError("completeRequest called more than once"); - log.log(Level.WARNING, "Assertion failed.", e); - throw e; - } - try { parent.async.complete(); log.finest(() -> "Request completed successfully: " + parent.servletRequest.getRequestURI()); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java index 271805765c2..7a15107a3eb 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletOutputStreamWriter.java @@ -41,6 +41,10 @@ public class ServletOutputStreamWriter { private static final Logger log = Logger.getLogger(ServletOutputStreamWriter.class.getName()); + // TODO: This reference is not guaranteed to be unique; ByteBuffer.allocate(0) MAY in principle return a singleton! + // If so, application code could fake a close by writing such a byte buffer. + // The problem can be solved by filtering out zero-length byte buffers from application code. + // Other ways to express this are also possible, e.g. with a 'closed' state checked when queue goes empty. private static final ByteBuffer CLOSE_STREAM_BUFFER = ByteBuffer.allocate(0); private final Object monitor = new Object(); @@ -74,6 +78,7 @@ public class ServletOutputStreamWriter { public void setSendingError() { synchronized (monitor) { + // TODO: This assert seems fishy. Investigate. assertStateIs(state, State.NOT_STARTED); state = State.FINISHED_OR_ERROR; } @@ -109,6 +114,7 @@ public class ServletOutputStreamWriter { } if (thisThreadShouldWrite) { + // TODO: Consider refactoring to avoid multiple monitor entry-exit. writeBuffersInQueueToOutputStream(); } } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java index 5bea01bd104..a763a03d39d 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletRequestReader.java @@ -133,9 +133,8 @@ class ServletRequestReader implements ReadListener { numberOfOutstandingUserCalls += 2; } try { - requestContentChannel.write(buf, writeCompletionHandler); - int bytesReceived = buf.remaining(); + requestContentChannel.write(buf, writeCompletionHandler); metricReporter.successfulRead(bytesReceived); } catch (final Throwable t) { finishedFuture.completeExceptionally(t); diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java index b0781c402d5..126e4fee9e6 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/ServletResponseController.java @@ -115,6 +115,8 @@ public class ServletResponseController { } try { + // TODO: sendError() is a synchronous call. Refactor. (Also, we should control the response content - + // this method generates a response body based on Jetty's own response templates ("Powered by Jetty"). servletResponse.sendError( statusCode, reasonPhrase); @@ -166,10 +168,14 @@ public class ServletResponseController { private static void setStatus_holdingLock(Response jdiscResponse, HttpServletResponse servletResponse) { if (jdiscResponse instanceof HttpResponse) { + // TODO: Figure out what this does to the response (with Jetty), and move to non-deprecated APIs. + // Deprecate our own code as necessary. servletResponse.setStatus(jdiscResponse.getStatus(), ((HttpResponse) jdiscResponse).getMessage()); } else { Optional<String> errorMessage = getErrorMessage(jdiscResponse); if (errorMessage.isPresent()) { + // TODO: Figure out what this does to the response (with Jetty), and move to non-deprecated APIs. + // Deprecate our own code as necessary. servletResponse.setStatus(jdiscResponse.getStatus(), errorMessage.get()); } else { servletResponse.setStatus(jdiscResponse.getStatus()); |