Issue #4376 Async Content Complete

Added test harness to reproduce unready completing write. Fixed test by not closing output prior to becoming READY Signed-off-by: Greg Wilkins <gregw@webtide.com>
jetty · Nov 28, 2019 · b30755f · b30755f
1 parent d7cf372
commit b30755f
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 12 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
@@ -1195,10 +1195,7 @@ public void succeeded()
             if (_length > 0)
                 _combinedListener.onResponseContent(_request, _content);
             if (_complete && _state.completeResponse())
-            {
-                _response.getHttpOutput().closed();
                 _combinedListener.onResponseEnd(_request);
-            }
             super.succeeded();
         }
 
@@ -1222,7 +1219,6 @@ public void succeeded()
                     @Override
                     public void failed(Throwable th)
                     {
-                        _response.getHttpOutput().closed();
                         abort(x);
                         super.failed(x);
                     }

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
@@ -323,6 +323,7 @@ public void close()
             {
                 case CLOSING:
                 case CLOSED:
+                case ERROR:
                 {
                     _closeCallback = null;
                     closeCallback.succeeded();
@@ -403,21 +404,19 @@ public void closed()
             State state = _state.get();
             switch (state)
             {
-                case CLOSING:
-                {
-                    if (!_state.compareAndSet(state, State.CLOSED))
-                        break;
-                    releaseBuffer();
-                    return;
-                }
                 case CLOSED:
                 {
                     return;
                 }
                 case UNREADY:
                 {
                     if (_state.compareAndSet(state, State.ERROR))
-                        _writeListener.onError(_onError == null ? new EofException("Async closed") : _onError);
+                    {
+                        if (_onError == null)
+                            _onError = new EofException("Async closed");
+                        releaseBuffer();
+                        return;
+                    }
                     break;
                 }
                 default:

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
@@ -33,6 +33,12 @@
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Stream;
+import javax.servlet.AsyncContext;
+import javax.servlet.ServletException;
+import javax.servlet.ServletOutputStream;
+import javax.servlet.WriteListener;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 import org.eclipse.jetty.http.HttpCompliance;
 import org.eclipse.jetty.http.HttpTester;
@@ -41,6 +47,7 @@
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.ManagedSelector;
 import org.eclipse.jetty.io.SocketChannelEndPoint;
+import org.eclipse.jetty.server.handler.AbstractHandler;
 import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.util.thread.Scheduler;
 import org.hamcrest.Matchers;
@@ -164,6 +171,7 @@ public static Stream<Arguments> tests()
         List<Object[]> tests = new ArrayList<>();
         tests.add(new Object[]{new HelloWorldHandler(), 200, "Hello world"});
         tests.add(new Object[]{new SendErrorHandler(499,"Test async sendError"), 499, "Test async sendError"});
+        tests.add(new Object[]{new AsyncReadyCompleteHandler(), 200, AsyncReadyCompleteHandler.data});
         return tests.stream().map(Arguments::of);
     }
 
@@ -218,4 +226,44 @@ public void testAsyncCompletion(Handler handler, int status, String message) thr
             }
         }
     }
+
+    private static class AsyncReadyCompleteHandler extends AbstractHandler
+    {
+        static String data = "Now is the time for all good men to come to the aid of the party";
+
+        @Override
+        public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+        {
+            AsyncContext context = request.startAsync();
+            ServletOutputStream out = response.getOutputStream();
+            out.setWriteListener(new WriteListener() {
+                byte[] bytes = data.getBytes(StandardCharsets.ISO_8859_1);
+                @Override
+                public void onWritePossible() throws IOException
+                {
+                    while (out.isReady())
+                    {
+                        if (bytes != null)
+                        {
+                            response.setContentType("text/plain");
+                            response.setContentLength(bytes.length);
+                            out.write(bytes);
+                            bytes = null;
+                        }
+                        else
+                        {
+                            context.complete();
+                            return;
+                        }
+                    }
+                }
+
+                @Override
+                public void onError(Throwable t)
+                {
+                    t.printStackTrace();
+                }
+            });
+        }
+    }
 }