Added some TODOs to the code.

2 files changed, 12 insertions, 0 deletions

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/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());