Eclipse
/
jetty.project
mirror of https://github.com/jetty/jetty.project.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

461452 Double release of buffer by HttpReceiverOverHTTP 

Updated HttpParse to always return from parseNext when messageComplete is called.  This allows it to notice reentrant state changes
and removes the need for HttpReceiverOverHTTP.methodComplete to return true, thus avoiding the race.
This commit is contained in:
Greg Wilkins 2015-03-05 15:26:19 +11:00
parent 418a60bbd0
commit 042de2ec9f
5 changed files with 19 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
jetty-client/src/main/java/org/eclipse/jetty/client/ContinueProtocolHandler.java
View File

@ -78,7 +78,7 @@ public class ContinueProtocolHandler implements ProtocolHandler
case 100:
{
// All good, continue
exchange.resetResponse(true);
exchange.resetResponse();
exchange.proceed(null);
break;
}

7
jetty-client/src/main/java/org/eclipse/jetty/client/HttpExchange.java
View File

@ -220,13 +220,10 @@ public class HttpExchange
}
}
public void resetResponse(boolean success)
public void resetResponse()
{
responseComplete.set(false);
int responseSuccess = 0b1100;
int responseFailure = 0b0100;
int code = success ? responseSuccess : responseFailure;
complete.addAndGet(-code);
complete.addAndGet(-0b1100);
}
public void proceed(Throwable failure)

29
jetty-client/src/main/java/org/eclipse/jetty/client/http/HttpReceiverOverHTTP.java
View File

@ -151,26 +151,7 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
if (LOG.isDebugEnabled())
LOG.debug("Parsed {}, remaining {} {}", handle, buffer.remaining(), parser);
if (handle)
{
if (!parser.isStart())
{
// The parser has NOT been reset.
// So we have not ourselves parsed the messageComplete() OR if we did we failed the atomic tests in
// responseComplete and the parser is not yet reset, but soon will be by another thread.
// So most likely we are here because async handling is underway and a callback will eventually call resume
// which will call process to progress. So we can return true here to stop us calling process.
return true;
}
// The parser has been reset! If this was done by this thread parsing message Complete, then returning false here
// will let the parsing continue, fill 0 and return the buffer. All good.
// If the parser was reset by an asynchronous callback calling resume that called process(), then it may have already
// returned false to that thread, which may have already filled 0 and thus already returned the buffer
// TODO This is the double return race.
}
return false;
return handle;
}
protected void fillInterested()
@ -273,11 +254,9 @@ public class HttpReceiverOverHTTP extends HttpReceiver implements HttpParser.Res
public boolean messageComplete()
{
HttpExchange exchange = getHttpExchange();
if (exchange == null)
return false;
responseSuccess(exchange);
return true;
if (exchange != null)
responseSuccess(exchange);
return false;
}
@Override

28
jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View File

@ -628,6 +628,7 @@ public class HttpParser
BufferUtil.clear(buffer);
handle=_handler.headerComplete()||handle;
handle=_handler.messageComplete()||handle;
return handle;
}
else
{
@ -723,6 +724,7 @@ public class HttpParser
BufferUtil.clear(buffer);
handle=_handler.headerComplete()||handle;
handle=_handler.messageComplete()||handle;
return handle;
}
}
else if (ch<0)
@ -1015,23 +1017,23 @@ public class HttpParser
case EOF_CONTENT:
setState(State.EOF_CONTENT);
handle=_handler.headerComplete()||handle;
break;
return handle;
case CHUNKED_CONTENT:
setState(State.CHUNKED_CONTENT);
handle=_handler.headerComplete()||handle;
break;
return handle;
case NO_CONTENT:
handle=_handler.headerComplete()||handle;
setState(State.END);
handle=_handler.messageComplete()||handle;
break;
return handle;
default:
setState(State.CONTENT);
handle=_handler.headerComplete()||handle;
break;
return handle;
}
}
else if (ch<=HttpTokens.SPACE)
@ -1262,8 +1264,7 @@ public class HttpParser
if (_responseStatus>0 && _headResponse)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
else
{
@ -1378,8 +1379,7 @@ public class HttpParser
if (content == 0)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
}
@ -1403,8 +1403,7 @@ public class HttpParser
if (content == 0)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
else
{
@ -1427,8 +1426,7 @@ public class HttpParser
if(_contentPosition == _contentLength)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
}
break;
@ -1457,8 +1455,7 @@ public class HttpParser
if (_chunkLength == 0)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
else
setState(State.CHUNK);
@ -1478,8 +1475,7 @@ public class HttpParser
if (_chunkLength == 0)
{
setState(State.END);
if (_handler.messageComplete())
return true;
return _handler.messageComplete();
}
else
setState(State.CHUNK);

2
jetty-http/src/test/java/org/eclipse/jetty/http/HttpParserTest.java
View File

@ -79,8 +79,6 @@ public class HttpParserTest
BufferUtil.append(b,BufferUtil.toBuffer(" "));
assertEquals(HttpMethod.GET,HttpMethod.lookAheadGet(b));
}