From b30755ff2053d6a594bceea576c4253513d1feef Mon Sep 17 00:00:00 2001
From: Greg Wilkins <gregw@webtide.com>
Date: Fri, 29 Nov 2019 10:16:34 +1100
Subject: [PATCH] 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>
---
 .../org/eclipse/jetty/server/HttpChannel.java |  4 --
 .../org/eclipse/jetty/server/HttpOutput.java  | 15 +++---
 .../jetty/server/AsyncCompletionTest.java     | 48 +++++++++++++++++++
 3 files changed, 55 insertions(+), 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
index 1f787a0356c9..9134588cfd99 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
@@ -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
index 70fbf4868e74..74d716ce5d0d 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
@@ -323,6 +323,7 @@ public void close()
             {
                 case CLOSING:
                 case CLOSED:
+                case ERROR:
                 {
                     _closeCallback = null;
                     closeCallback.succeeded();
@@ -403,13 +404,6 @@ public void closed()
             State state = _state.get();
             switch (state)
             {
-                case CLOSING:
-                {
-                    if (!_state.compareAndSet(state, State.CLOSED))
-                        break;
-                    releaseBuffer();
-                    return;
-                }
                 case CLOSED:
                 {
                     return;
@@ -417,7 +411,12 @@ public void closed()
                 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
index a14a6bcd09f4..bdcefa7625bc 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
@@ -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();
+                }
+            });
+        }
+    }
 }