357240 ensure half closed is not schedule with no progress

This commit is contained in:
Greg Wilkins 2011-10-05 14:48:47 +11:00
parent be8edbd785
commit f778867f32
5 changed files with 54 additions and 49 deletions

View File

@ -483,8 +483,13 @@ public abstract class AbstractGenerator implements Generator
{ {
if (close) if (close)
_persistent=false; _persistent=false;
if (!isCommitted()) if (isCommitted())
{ {
LOG.debug("sendError on committed: {} {}",code,reason);
}
else
{
LOG.debug("sendError: {} {}",code,reason);
setResponse(code, reason); setResponse(code, reason);
if (content != null) if (content != null)
{ {

View File

@ -27,7 +27,6 @@ public class AsyncHttpConnection extends HttpConnection
public Connection handle() throws IOException public Connection handle() throws IOException
{ {
LOG.debug("handle {}",this);
Connection connection = this; Connection connection = this;
boolean some_progress=false; boolean some_progress=false;
boolean progress=true; boolean progress=true;
@ -60,6 +59,11 @@ public class AsyncHttpConnection extends HttpConnection
// Flush output from buffering endpoint // Flush output from buffering endpoint
if (_endp.isBufferingOutput()) if (_endp.isBufferingOutput())
_endp.flush(); _endp.flush();
// Special case close handling.
// If we were dispatched and have made no progress, but io is shutdown, then close
if (!progress && !some_progress && (_endp.isInputShutdown()||_endp.isOutputShutdown()))
_endp.close();
} }
catch (HttpException e) catch (HttpException e)
{ {
@ -123,13 +127,12 @@ public class AsyncHttpConnection extends HttpConnection
{ {
setCurrentConnection(null); setCurrentConnection(null);
_parser.returnBuffers(); _parser.returnBuffers();
// Are we write blocked
if (_generator.isCommitted() && !_generator.isComplete() && _endp.isOpen())
((AsyncEndPoint)_endp).scheduleWrite();
else
_generator.returnBuffers(); _generator.returnBuffers();
// Check if we are write blocked
if (_generator.isCommitted() && !_generator.isComplete() && _endp.isOpen() && !_endp.isOutputShutdown())
((AsyncEndPoint)_endp).scheduleWrite(); // TODO. This should not be required
if (!some_progress) if (!some_progress)
{ {
_total_no_progress++; _total_no_progress++;
@ -137,11 +140,6 @@ public class AsyncHttpConnection extends HttpConnection
if (NO_PROGRESS_INFO>0 && _total_no_progress%NO_PROGRESS_INFO==0 && (NO_PROGRESS_CLOSE<=0 || _total_no_progress< NO_PROGRESS_CLOSE)) if (NO_PROGRESS_INFO>0 && _total_no_progress%NO_PROGRESS_INFO==0 && (NO_PROGRESS_CLOSE<=0 || _total_no_progress< NO_PROGRESS_CLOSE))
{ {
LOG.info("EndPoint making no progress: "+_total_no_progress+" "+_endp); LOG.info("EndPoint making no progress: "+_total_no_progress+" "+_endp);
LOG.setDebugEnabled(true);
Log.getLogger("org.eclipse.jetty.io.nio").getLogger("ssl").setDebugEnabled(true);
Log.getLogger(ChannelEndPoint.class).setDebugEnabled(true);
} }
if (NO_PROGRESS_CLOSE>0 && _total_no_progress>NO_PROGRESS_CLOSE) if (NO_PROGRESS_CLOSE>0 && _total_no_progress>NO_PROGRESS_CLOSE)
@ -154,7 +152,6 @@ public class AsyncHttpConnection extends HttpConnection
} }
} }
} }
LOG.debug("unhandle {}",this);
} }
return connection; return connection;
} }

View File

@ -453,29 +453,29 @@ public abstract class HttpConnection extends AbstractConnection
catch (EofException e) catch (EofException e)
{ {
LOG.debug(e); LOG.debug(e);
_request.setHandled(true);
error=true; error=true;
_request.setHandled(true);
} }
catch (RuntimeIOException e) catch (RuntimeIOException e)
{ {
LOG.debug(e); LOG.debug(e);
_request.setHandled(true);
error=true; error=true;
_request.setHandled(true);
} }
catch (HttpException e) catch (HttpException e)
{ {
LOG.debug(e); LOG.debug(e);
error=true;
_request.setHandled(true); _request.setHandled(true);
_response.sendError(e.getStatus(), e.getReason()); _response.sendError(e.getStatus(), e.getReason());
error=true;
} }
catch (Throwable e) catch (Throwable e)
{ {
if (e instanceof ThreadDeath) if (e instanceof ThreadDeath)
throw (ThreadDeath)e; throw (ThreadDeath)e;
error=true;
LOG.warn(String.valueOf(_uri),e); LOG.warn(String.valueOf(_uri),e);
error=true;
_request.setHandled(true); _request.setHandled(true);
_generator.sendError(info==null?400:500, null, null, true); _generator.sendError(info==null?400:500, null, null, true);
} }
@ -509,7 +509,12 @@ public abstract class HttpConnection extends AbstractConnection
if(_endp.isOpen()) if(_endp.isOpen())
{ {
if (error) if (error)
{
_endp.shutdownOutput(); _endp.shutdownOutput();
_generator.setPersistent(false);
if (!_generator.isComplete())
_response.complete();
}
else else
{ {
if (!_response.isCommitted() && !_request.isHandled()) if (!_response.isCommitted() && !_request.isHandled())

View File

@ -37,8 +37,11 @@ import javax.servlet.http.HttpServletResponse;
import junit.framework.Assert; import junit.framework.Assert;
import org.eclipse.jetty.io.EndPoint;
import org.eclipse.jetty.server.handler.AbstractHandler; import org.eclipse.jetty.server.handler.AbstractHandler;
import org.eclipse.jetty.util.IO; import org.eclipse.jetty.util.IO;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.StdErrLog;
import org.junit.Test; import org.junit.Test;
/** /**
@ -108,7 +111,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest1() throws Exception public void testRequest1() throws Exception
{ {
System.err.println("testRequest1");
configureServer(new HelloWorldHandler()); configureServer(new HelloWorldHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -134,7 +136,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testFragmentedChunk() throws Exception public void testFragmentedChunk() throws Exception
{ {
System.err.println("testFragmentedChunk");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -168,7 +169,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest1Fragments() throws Exception, InterruptedException public void testRequest1Fragments() throws Exception, InterruptedException
{ {
System.err.println("testRequest1Fragments");
configureServer(new HelloWorldHandler()); configureServer(new HelloWorldHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -202,7 +202,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest2() throws Exception public void testRequest2() throws Exception
{ {
System.err.println("testRequest2");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
byte[] bytes=REQUEST2.getBytes(); byte[] bytes=REQUEST2.getBytes();
@ -232,7 +231,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest2Fragments() throws Exception public void testRequest2Fragments() throws Exception
{ {
System.err.println("testRequest2Fragments");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
byte[] bytes=REQUEST2.getBytes(); byte[] bytes=REQUEST2.getBytes();
@ -277,7 +275,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest2Iterate() throws Exception public void testRequest2Iterate() throws Exception
{ {
System.err.println("testRequest2Iterate");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
byte[] bytes=REQUEST2.getBytes(); byte[] bytes=REQUEST2.getBytes();
@ -317,7 +314,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRequest2KnownBad() throws Exception public void testRequest2KnownBad() throws Exception
{ {
System.err.println("testRequest2KnownBad");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
byte[] bytes=REQUEST2.getBytes(); byte[] bytes=REQUEST2.getBytes();
@ -356,7 +352,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testFlush() throws Exception public void testFlush() throws Exception
{ {
System.err.println("testFlush");
configureServer(new DataHandler()); configureServer(new DataHandler());
String[] encoding = {"NONE","UTF-8","ISO-8859-1","ISO-8859-2"}; String[] encoding = {"NONE","UTF-8","ISO-8859-1","ISO-8859-2"};
@ -393,7 +388,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testBlockingWhileReadingRequestContent() throws Exception public void testBlockingWhileReadingRequestContent() throws Exception
{ {
System.err.println("testBlockingWhileReadingRequestContent");
configureServer(new DataHandler()); configureServer(new DataHandler());
long start=System.currentTimeMillis(); long start=System.currentTimeMillis();
@ -452,7 +446,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testBlockingWhileWritingResponseContent() throws Exception public void testBlockingWhileWritingResponseContent() throws Exception
{ {
System.err.println("testBlockingWhileWritingResponseContent");
configureServer(new DataHandler()); configureServer(new DataHandler());
long start=System.currentTimeMillis(); long start=System.currentTimeMillis();
@ -498,7 +491,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testBigBlocks() throws Exception public void testBigBlocks() throws Exception
{ {
System.err.println("testBigBlocks");
configureServer(new BigBlockHandler()); configureServer(new BigBlockHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -632,7 +624,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testPipeline() throws Exception public void testPipeline() throws Exception
{ {
System.err.println("testPipeline");
configureServer(new HelloWorldHandler()); configureServer(new HelloWorldHandler());
//for (int pipeline=1;pipeline<32;pipeline++) //for (int pipeline=1;pipeline<32;pipeline++)
@ -689,7 +680,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRecycledWriters() throws Exception public void testRecycledWriters() throws Exception
{ {
System.err.println("testRecycledWriters");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -778,7 +768,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testHead() throws Exception public void testHead() throws Exception
{ {
System.err.println("testHead");
configureServer(new EchoHandler(false)); configureServer(new EchoHandler(false));
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -831,7 +820,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testRecycledReaders() throws Exception public void testRecycledReaders() throws Exception
{ {
System.err.println("testRecycledReaders");
configureServer(new EchoHandler()); configureServer(new EchoHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -891,7 +879,6 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testBlockedClient() throws Exception public void testBlockedClient() throws Exception
{ {
System.err.println("testBlockedClient");
configureServer(new HelloWorldHandler()); configureServer(new HelloWorldHandler());
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
@ -933,12 +920,13 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
@Test @Test
public void testCommittedError() throws Exception public void testCommittedError() throws Exception
{ {
System.err.println("testCommittedError"); CommittedErrorHandler handler =new CommittedErrorHandler();
configureServer(new CommittedErrorHandler()); configureServer(handler);
Socket client=newSocket(HOST,_connector.getLocalPort()); Socket client=newSocket(HOST,_connector.getLocalPort());
try try
{ {
((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(true);
OutputStream os=client.getOutputStream(); OutputStream os=client.getOutputStream();
InputStream is=client.getInputStream(); InputStream is=client.getInputStream();
@ -946,37 +934,45 @@ public abstract class HttpServerTestBase extends HttpServerTestFixture
os.write(( os.write((
"GET / HTTP/1.1\r\n"+ "GET / HTTP/1.1\r\n"+
"Host: "+HOST+":"+_connector.getLocalPort()+"\r\n" + "Host: "+HOST+":"+_connector.getLocalPort()+"\r\n" +
"Connection: close\r\n"+
"\r\n" "\r\n"
).getBytes()); ).getBytes());
os.flush(); os.flush();
client.setSoTimeout(2000); client.setSoTimeout(2000);
String in = IO.toString(is); String in = IO.toString(is);
System.err.println("in="+in);
Thread.sleep(2000); assertEquals(-1,is.read()); // Closed by error!
assertTrue(in.indexOf("HTTP/1.1 200 OK")>=0);
assertTrue(in.indexOf("Transfer-Encoding: chunked")>0);
assertTrue(in.indexOf("Now is the time for all good men to come to the aid of the party")>0);
assertTrue(in.indexOf("\r\n0\r\n")==-1); // chunking is interrupted by error close
assertTrue(!handler._endp.isBlocking() || handler._endp.isOutputShutdown()); // oshut
client.close();
Thread.sleep(100);
assertTrue(!handler._endp.isOpen());
} }
finally finally
{ {
System.err.println("closing"); ((StdErrLog)Log.getLogger(HttpConnection.class)).setHideStacks(false);
client.close();
Thread.sleep(2000);
System.err.println("FINALLY"); if (!client.isClosed())
Thread.sleep(2000); client.close();
} }
} }
protected static class CommittedErrorHandler extends AbstractHandler protected static class CommittedErrorHandler extends AbstractHandler
{ {
public EndPoint _endp;
public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException public void handle(String target, Request baseRequest, HttpServletRequest request, HttpServletResponse response) throws IOException, ServletException
{ {
_endp=baseRequest.getConnection().getEndPoint();
response.setHeader("test","value"); response.setHeader("test","value");
response.setStatus(200); response.setStatus(200);
response.setContentType("text/plain"); response.setContentType("text/plain");
response.getWriter().println("Now is the time for all good ment to come to the aid of the party"); response.getWriter().println("Now is the time for all good men to come to the aid of the party");
response.getWriter().flush(); response.getWriter().flush();
response.flushBuffer(); response.flushBuffer();

View File

@ -27,8 +27,10 @@ public class SelectChannelServerTest extends HttpServerTestBase
} }
@Override @Override
public void testBigBlocks() throws Exception public void testCommittedError() throws Exception
{ {
super.testBigBlocks(); super.testCommittedError();
} }
} }