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 3da6c658ce6..b4849407f1d 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 @@ -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; diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ServletWriterTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ServletWriterTest.java new file mode 100644 index 00000000000..3728880a5f6 --- /dev/null +++ b/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 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; + } + } +}