[WIP!] HTTP Close #1832 (#2338)

* Only close if parser closed and output is shutdown

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* a better possible fix

Signed-off-by: Greg Wilkins <gregw@webtide.com>

* after review

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-03-15 23:12:58 +11:00 committed by GitHub
parent 5b8fe10154
commit 1da19dc2fd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 18 additions and 12 deletions

View File

@ -62,6 +62,8 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint
protected final void shutdownInput() protected final void shutdownInput()
{ {
if (LOG.isDebugEnabled())
LOG.debug("shutdownInput {}",this);
while(true) while(true)
{ {
State s = _state.get(); State s = _state.get();
@ -114,6 +116,8 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint
@Override @Override
public final void shutdownOutput() public final void shutdownOutput()
{ {
if (LOG.isDebugEnabled())
LOG.debug("shutdownOutput {}",this);
while(true) while(true)
{ {
State s = _state.get(); State s = _state.get();
@ -166,11 +170,15 @@ public abstract class AbstractEndPoint extends IdleTimeout implements EndPoint
@Override @Override
public final void close() public final void close()
{ {
if (LOG.isDebugEnabled())
LOG.debug("close {}",this);
close(null); close(null);
} }
protected final void close(Throwable failure) protected final void close(Throwable failure)
{ {
if (LOG.isDebugEnabled())
LOG.debug("close({}) {}",failure,this);
while(true) while(true)
{ {
State s = _state.get(); State s = _state.get();

View File

@ -496,6 +496,8 @@ abstract public class WriteFlusher
public boolean onFail(Throwable cause) public boolean onFail(Throwable cause)
{ {
// Keep trying to handle the failure until we get to IDLE or FAILED state // Keep trying to handle the failure until we get to IDLE or FAILED state
if (cause!=null)
cause.addSuppressed(new Throwable());
while (true) while (true)
{ {
State current = _state.get(); State current = _state.get();

View File

@ -243,6 +243,8 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
int filled = fillRequestBuffer(); int filled = fillRequestBuffer();
if (filled>0) if (filled>0)
bytesIn.add(filled); bytesIn.add(filled);
else if (filled==-1 && getEndPoint().isOutputShutdown())
close();
// Parse the request buffer. // Parse the request buffer.
boolean handle = parseRequestBuffer(); boolean handle = parseRequestBuffer();
@ -252,13 +254,6 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
if (getEndPoint().getConnection()!=this) if (getEndPoint().getConnection()!=this)
break; break;
// Handle closed parser.
if (_parser.isClose() || _parser.isClosed())
{
close();
break;
}
// Handle channel event // Handle channel event
if (handle) if (handle)
{ {

View File

@ -26,6 +26,7 @@ import java.io.IOException;
import java.io.InputStream; import java.io.InputStream;
import java.io.OutputStream; import java.io.OutputStream;
import java.net.Socket; import java.net.Socket;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets; import java.nio.charset.StandardCharsets;
import java.util.concurrent.CountDownLatch; import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
@ -42,9 +43,11 @@ import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.toolchain.test.AdvancedRunner; import org.eclipse.jetty.toolchain.test.AdvancedRunner;
import org.eclipse.jetty.toolchain.test.MavenTestingUtils; import org.eclipse.jetty.toolchain.test.MavenTestingUtils;
import org.eclipse.jetty.toolchain.test.annotation.Slow; import org.eclipse.jetty.toolchain.test.annotation.Slow;
import org.eclipse.jetty.util.BufferUtil;
import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.resource.Resource; import org.eclipse.jetty.util.resource.Resource;
import org.eclipse.jetty.util.ssl.SslContextFactory; import org.eclipse.jetty.util.ssl.SslContextFactory;
import org.hamcrest.Matchers;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.junit.runner.RunWith; import org.junit.runner.RunWith;
@ -224,11 +227,9 @@ public class ConnectionOpenCloseTest extends AbstractHttpTest
"\r\n").getBytes(StandardCharsets.UTF_8)); "\r\n").getBytes(StandardCharsets.UTF_8));
output.flush(); output.flush();
InputStream inputStream = socket.getInputStream(); // Read to EOF
HttpTester.Response response = HttpTester.parseResponse(inputStream); String response = BufferUtil.toString(ByteBuffer.wrap(IO.readBytes(socket.getInputStream())));
Assert.assertEquals(200, response.getStatus()); assertThat(response,Matchers.containsString("200 OK"));
Assert.assertEquals(-1, inputStream.read());
socket.close(); socket.close();
Assert.assertTrue(openLatch.await(5, TimeUnit.SECONDS)); Assert.assertTrue(openLatch.await(5, TimeUnit.SECONDS));