446559 Avoid spin consuming extra data

This commit is contained in:
Greg Wilkins 2014-10-10 13:16:41 +11:00
parent 0df54fd279
commit cd15bb187d
3 changed files with 79 additions and 27 deletions

View File

@ -25,6 +25,7 @@ import static org.eclipse.jetty.http.HttpTokens.TAB;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.EnumSet;
import org.eclipse.jetty.http.HttpTokens.EndOfContent;
import org.eclipse.jetty.util.ArrayTernaryTrie;
@ -123,9 +124,13 @@ public class HttpParser
CHUNK_PARAMS,
CHUNK,
END,
CLOSED
CLOSE, // The associated stream/endpoint should be closed
CLOSED // The associated stream/endpoint is at EOF
}
private final static EnumSet<State> __idleStates = EnumSet.of(State.START,State.END,State.CLOSE,State.CLOSED);
private final static EnumSet<State> __completeStates = EnumSet.of(State.END,State.CLOSE,State.CLOSED);
private final boolean DEBUG=LOG.isDebugEnabled(); // Cache debug to help branch prediction
private final HttpHandler _handler;
private final RequestHandler _requestHandler;
@ -144,7 +149,6 @@ public class HttpParser
/* ------------------------------------------------------------------------------- */
private volatile State _state=State.START;
private volatile boolean _eof;
private volatile boolean _closed;
private HttpMethod _method;
private String _methodString;
private HttpVersion _version;
@ -313,6 +317,12 @@ public class HttpParser
return isState(State.START);
}
/* ------------------------------------------------------------ */
public boolean isClose()
{
return isState(State.CLOSE);
}
/* ------------------------------------------------------------ */
public boolean isClosed()
{
@ -322,13 +332,13 @@ public class HttpParser
/* ------------------------------------------------------------ */
public boolean isIdle()
{
return isState(State.START)||isState(State.END)||isState(State.CLOSED);
return __idleStates.contains(_state);
}
/* ------------------------------------------------------------ */
public boolean isComplete()
{
return isState(State.END)||isState(State.CLOSED);
return __completeStates.contains(_state);
}
/* ------------------------------------------------------------------------------- */
@ -797,10 +807,8 @@ public class HttpParser
case CONNECTION:
// Don't cache if not persistent
if (_valueString!=null && _valueString.contains("close"))
{
_closed=true;
_connectionFields=null;
}
break;
case AUTHORIZATION:
@ -1161,7 +1169,7 @@ public class HttpParser
while (buffer.remaining()>0 && buffer.get(buffer.position())<=HttpTokens.SPACE)
buffer.get();
}
else if (_state==State.CLOSED)
else if (_state==State.CLOSE || _state==State.CLOSED)
{
if (BufferUtil.hasContent(buffer))
{
@ -1190,6 +1198,7 @@ public class HttpParser
break;
case END:
case CLOSE:
setState(State.CLOSED);
break;
@ -1214,8 +1223,6 @@ public class HttpParser
break;
}
}
return false;
}
catch(BadMessageException e)
{
@ -1229,28 +1236,25 @@ public class HttpParser
LOG.warn("bad HTTP parsed: "+e._code+(e.getReason()!=null?" "+e.getReason():"")+" for "+_handler,e);
else
LOG.warn("bad HTTP parsed: "+e._code+(e.getReason()!=null?" "+e.getReason():"")+" for "+_handler);
setState(State.CLOSED);
setState(State.CLOSE);
_handler.badMessage(e.getCode(), e.getReason());
return false;
}
catch(NumberFormatException|IllegalStateException e)
{
BufferUtil.clear(buffer);
LOG.warn("parse exception: "+e.toString()+" for "+_handler);
LOG.warn("parse exception: {} in {} for {}",e.toString(),_state,_handler);
if (DEBUG)
LOG.debug(e);
if (_state.ordinal()<=State.END.ordinal())
{
setState(State.CLOSED);
setState(State.CLOSE);
_handler.badMessage(400,null);
}
else
{
_handler.earlyEOF();
setState(State.CLOSED);
setState(State.CLOSE);
}
return false;
}
catch(Exception|Error e)
{
@ -1260,17 +1264,16 @@ public class HttpParser
if (_state.ordinal()<=State.END.ordinal())
{
setState(State.CLOSED);
setState(State.CLOSE);
_handler.badMessage(400,null);
}
else
{
_handler.earlyEOF();
setState(State.CLOSED);
setState(State.CLOSE);
}
return false;
}
return false;
}
protected boolean parseContent(ByteBuffer buffer)
@ -1439,8 +1442,9 @@ public class HttpParser
}
/* ------------------------------------------------------------------------------- */
/** Signal that the associated data source is at EOF
*/
public void atEOF()
{
if (DEBUG)
LOG.debug("atEOF {}", this);
@ -1448,11 +1452,13 @@ public class HttpParser
}
/* ------------------------------------------------------------------------------- */
/** Request that the associated data source be closed
*/
public void close()
{
if (DEBUG)
LOG.debug("close {}", this);
setState(State.CLOSED);
setState(State.CLOSE);
}
/* ------------------------------------------------------------------------------- */
@ -1460,10 +1466,11 @@ public class HttpParser
{
if (DEBUG)
LOG.debug("reset {}", this);
// reset state
if (_state==State.CLOSED)
return;
if (_closed)
if (_state==State.CLOSE)
{
setState(State.CLOSED);
return;

View File

@ -1030,11 +1030,15 @@ public class HttpParserTest
assertEquals(null,_content);
assertTrue(_headerCompleted);
assertTrue(_messageCompleted);
parser.close();
parser.reset();
parser.parseNext(buffer);
assertFalse(buffer.hasRemaining());
assertTrue(parser.isClosed());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1055,6 +1059,9 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("No URI",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1075,6 +1082,9 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("No URI",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1094,7 +1104,11 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("Unknown Version",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@Test
@ -1113,6 +1127,9 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("No Status",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1132,6 +1149,9 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("No Status",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1151,7 +1171,10 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("Unknown Version",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSED,parser.getState());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
buffer= BufferUtil.toBuffer(
"GET / HTTP/1.01\015\012"
@ -1166,6 +1189,9 @@ public class HttpParserTest
assertEquals(null,_methodOrVersion);
assertEquals("Unknown Version",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1184,6 +1210,9 @@ public class HttpParserTest
parser.parseNext(buffer);
assertEquals("Bad EOL",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
@ -1199,6 +1228,9 @@ public class HttpParserTest
parser.parseNext(buffer);
assertEquals("Bad EOL",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1221,6 +1253,9 @@ public class HttpParserTest
assertEquals("GET",_methodOrVersion);
assertEquals("Bad Content-Length",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1240,6 +1275,9 @@ public class HttpParserTest
assertEquals("GET",_methodOrVersion);
assertEquals("Bad Content-Length",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}
@ -1259,6 +1297,9 @@ public class HttpParserTest
assertEquals("GET",_methodOrVersion);
assertEquals("Bad Content-Length",_bad);
assertFalse(buffer.hasRemaining());
assertEquals(HttpParser.State.CLOSE,parser.getState());
parser.atEOF();
parser.parseNext(BufferUtil.EMPTY_BUFFER);
assertEquals(HttpParser.State.CLOSED,parser.getState());
}

View File

@ -245,6 +245,10 @@ public class HttpConnection extends AbstractConnection implements Runnable, Http
// We are not in a race here for the request buffer as we have not yet received a request,
// so there are not an possible legal threads calling #parseContent or #completed.
releaseRequestBuffer();
// But we may have parsed an error condition, so let's check for close
if (_parser.isClose())
close();
}
}
}