Merge pull request #4200 from eclipse/jetty-9.4.x-4190-deadlock_httpoutput_close

Fixes #4190 Jetty hangs after thread blocked in SharedBlockingCallbac…

jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java

@@ -403,6 +403,13 @@ public class HttpOutput extends ServletOutputStream implements Runnable
             State state = _state.get();
             switch (state)
             {
+                case CLOSING:
+                {
+                    if (!_state.compareAndSet(state, State.CLOSED))
+                        break;
+                    releaseBuffer();
+                    return;
+                }
                 case CLOSED:
                 {
                     return;

jetty-server/src/test/java/org/eclipse/jetty/server/ServletWriterTest.java

@@ -0,0 +1,127 @@
+//
+//  ========================================================================
+//  Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd.
+//  ------------------------------------------------------------------------
+//  All rights reserved. This program and the accompanying materials
+//  are made available under the terms of the Eclipse Public License v1.0
+//  and Apache License v2.0 which accompanies this distribution.
+//
+//      The Eclipse Public License is available at
+//      http://www.eclipse.org/legal/epl-v10.html
+//
+//      The Apache License v2.0 is available at
+//      http://www.opensource.org/licenses/apache2.0.php
+//
+//  You may elect to redistribute this code under either of these licenses.
+//  ========================================================================
+//
+
+package org.eclipse.jetty.server;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.io.PrintWriter;
+import java.net.Socket;
+import java.util.Arrays;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.eclipse.jetty.server.handler.AbstractHandler;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class ServletWriterTest
+{
+    private Server server;
+    private ServerConnector connector;
+
+    private void start(int aggregationSize, Handler handler) throws Exception
+    {
+        server = new Server();
+        HttpConfiguration httpConfig = new HttpConfiguration();
+        httpConfig.setOutputBufferSize(2 * aggregationSize);
+        httpConfig.setOutputAggregationSize(2 * aggregationSize);
+        connector = new ServerConnector(server, 1, 1, new HttpConnectionFactory(httpConfig));
+        server.addConnector(connector);
+        server.setHandler(handler);
+        server.start();
+    }
+
+    @AfterEach
+    public void dispose() throws Exception
+    {
+        server.stop();
+    }
+
+    @Test
+    public void testTCPCongestedCloseDoesNotDeadlock() throws Exception
+    {
+        // Write a large content so it gets TCP congested when calling close().
+        char[] chars = new char[128 * 1024 * 1024];
+        CountDownLatch latch = new CountDownLatch(1);
+        AtomicReference<Thread> serverThreadRef = new AtomicReference<>();
+        start(chars.length, new AbstractHandler.ErrorDispatchHandler() {
+            @Override
+            protected void doNonErrorHandle(String target, Request jettyRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
+            {
+                serverThreadRef.set(Thread.currentThread());
+                jettyRequest.setHandled(true);
+                response.setContentType("text/plain; charset=utf-8");
+                PrintWriter writer = response.getWriter();
+                Arrays.fill(chars, '0');
+                // The write is entirely buffered.
+                writer.write(chars);
+                latch.countDown();
+                // Closing will trigger the write over the network.
+                writer.close();
+            }
+        });
+
+        try (Socket socket = new Socket("localhost", connector.getLocalPort()))
+        {
+            String request = "GET / HTTP/1.1\r\n" +
+                "Host: localhost\r\n" +
+                "Connection: close\r\n" +
+                "\r\n";
+            OutputStream output = socket.getOutputStream();
+            output.write(request.getBytes(UTF_8));
+            output.flush();
+
+            // Wait until the response is buffered, so close() will write it.
+            assertTrue(latch.await(5, TimeUnit.SECONDS));
+            // Don't read the response yet to trigger TCP congestion.
+            Thread.sleep(1000);
+
+            // Now read the response.
+            socket.setSoTimeout(5000);
+            InputStream input = socket.getInputStream();
+            BufferedReader reader = new BufferedReader(new InputStreamReader(input, UTF_8));
+            String line = reader.readLine();
+            assertThat(line, containsString(" 200 "));
+            // Consume all the content, we should see EOF.
+            while (line != null)
+            {
+                line = reader.readLine();
+            }
+        }
+        catch (Throwable x)
+        {
+            Thread thread = serverThreadRef.get();
+            if (thread != null)
+                thread.interrupt();
+            throw x;
+        }
+    }
+}