From 26bf17f9c1768df27151fe0dfd123eac9ff75150 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 14 Nov 2011 12:08:21 +1100 Subject: [PATCH] removed lock from HttpParser. Altered way that returnBuffers called to avoid nulling buffer for other thread --- .../org/eclipse/jetty/http/HttpParser.java | 123 ++++++++---------- .../jetty/server/AsyncHttpConnection.java | 9 +- .../jetty/server/AsyncRequestReadTest.java | 3 +- 3 files changed, 63 insertions(+), 72 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index ccccfb83b92..f1725ccc64f 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -83,8 +83,6 @@ public class HttpParser implements Parser protected int _chunkPosition; private boolean _headResponse; - private Lock _lock = new ReentrantLock(); // Ensure only a single parsing/resetting thread - /* ------------------------------------------------------------------------------- */ /** * Constructor. @@ -252,7 +250,6 @@ public class HttpParser implements Parser */ public int parseNext() throws IOException { - _lock.lock(); try { int progress=0; @@ -278,6 +275,7 @@ public class HttpParser implements Parser { _state=STATE_END; _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } @@ -340,6 +338,7 @@ public class HttpParser implements Parser if (!isComplete() && !isIdle()) throw new EofException(); + returnBuffers(); return -1; } length=_buffer.length(); @@ -453,6 +452,7 @@ public class HttpParser implements Parser _state=STATE_SEEKING_EOF; _handler.headerComplete(); _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } break; @@ -482,6 +482,7 @@ public class HttpParser implements Parser _state=STATE_SEEKING_EOF; _handler.headerComplete(); _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } } @@ -645,7 +646,8 @@ public class HttpParser implements Parser _handler.headerComplete(); _state=_persistent||(_responseStatus>=100&&_responseStatus<200)?STATE_END:STATE_SEEKING_EOF; _handler.messageComplete(_contentPosition); - break; + returnBuffers(); + return 1; default: _state=STATE_CONTENT; @@ -850,6 +852,7 @@ public class HttpParser implements Parser { _state=_persistent?STATE_END:STATE_SEEKING_EOF; _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } @@ -869,6 +872,7 @@ public class HttpParser implements Parser { _state=_persistent?STATE_END:STATE_SEEKING_EOF; _handler.messageComplete(_contentPosition); + returnBuffers(); } // TODO adjust the _buffer to keep unconsumed content return 1; @@ -903,6 +907,7 @@ public class HttpParser implements Parser _eol=_buffer.get(); _state=_persistent?STATE_END:STATE_SEEKING_EOF; _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } else @@ -933,6 +938,7 @@ public class HttpParser implements Parser _eol=_buffer.get(); _state=_persistent?STATE_END:STATE_SEEKING_EOF; _handler.messageComplete(_contentPosition); + returnBuffers(); return 1; } else @@ -979,10 +985,6 @@ public class HttpParser implements Parser _state=STATE_SEEKING_EOF; throw e; } - finally - { - _lock.unlock(); - } } /* ------------------------------------------------------------------------------- */ @@ -1048,85 +1050,70 @@ public class HttpParser implements Parser /* ------------------------------------------------------------------------------- */ public void reset() { - _lock.lock(); - try + // reset state + _contentView.setGetIndex(_contentView.putIndex()); + _state=_persistent?STATE_START:(_endp.isInputShutdown()?STATE_END:STATE_SEEKING_EOF); + _contentLength=HttpTokens.UNKNOWN_CONTENT; + _contentPosition=0; + _length=0; + _responseStatus=0; + + // Consume LF if CRLF + if (_eol == HttpTokens.CARRIAGE_RETURN && _buffer!=null && _buffer.hasContent() && _buffer.peek() == HttpTokens.LINE_FEED) + _eol=_buffer.get(); + + if (_body!=null && _body.hasContent()) { - // reset state - _contentView.setGetIndex(_contentView.putIndex()); - _state=_persistent?STATE_START:(_endp.isInputShutdown()?STATE_END:STATE_SEEKING_EOF); - _contentLength=HttpTokens.UNKNOWN_CONTENT; - _contentPosition=0; - _length=0; - _responseStatus=0; - - // Consume LF if CRLF - if (_eol == HttpTokens.CARRIAGE_RETURN && _buffer!=null && _buffer.hasContent() && _buffer.peek() == HttpTokens.LINE_FEED) - _eol=_buffer.get(); - - if (_body!=null && _body.hasContent()) + // There is content in the body after the end of the request. + // This is probably a pipelined header of the next request, so we need to + // copy it to the header buffer. + if (_header==null) { - // There is content in the body after the end of the request. - // This is probably a pipelined header of the next request, so we need to - // copy it to the header buffer. - if (_header==null) - { - _header=_buffers.getHeader(); - } - else - { - _header.setMarkIndex(-1); - _header.compact(); - } - int take=_header.space(); - if (take>_body.length()) - take=_body.length(); - _body.peek(_body.getIndex(),take); - _body.skip(_header.put(_body.peek(_body.getIndex(),take))); + _header=_buffers.getHeader(); } - - if (_header!=null) + else { _header.setMarkIndex(-1); _header.compact(); } - if (_body!=null) - _body.setMarkIndex(-1); + int take=_header.space(); + if (take>_body.length()) + take=_body.length(); + _body.peek(_body.getIndex(),take); + _body.skip(_header.put(_body.peek(_body.getIndex(),take))); + } - _buffer=_header; - } - finally + if (_header!=null) { - _lock.unlock(); + _header.setMarkIndex(-1); + _header.compact(); } + if (_body!=null) + _body.setMarkIndex(-1); + + _buffer=_header; + returnBuffers(); } /* ------------------------------------------------------------------------------- */ public void returnBuffers() { - _lock.lock(); - try + if (_body!=null && !_body.hasContent() && _body.markIndex()==-1 && _buffers!=null) { - if (_body!=null && !_body.hasContent() && _body.markIndex()==-1 && _buffers!=null) - { - if (_buffer==_body) - _buffer=_header; - if (_buffers!=null) - _buffers.returnBuffer(_body); - _body=null; - } - - if (_header!=null && !_header.hasContent() && _header.markIndex()==-1 && _buffers!=null) - { - if (_buffer==_header) - _buffer=null; - _buffers.returnBuffer(_header); - _header=null; - } + if (_buffer==_body) + _buffer=_header; + if (_buffers!=null) + _buffers.returnBuffer(_body); + _body=null; } - finally + + if (_header!=null && !_header.hasContent() && _header.markIndex()==-1 && _buffers!=null) { - _lock.unlock(); + if (_buffer==_header) + _buffer=null; + _buffers.returnBuffer(_header); + _header=null; } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncHttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncHttpConnection.java index 7ef6d99d0bf..36edce2440f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncHttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/AsyncHttpConnection.java @@ -125,9 +125,12 @@ public class AsyncHttpConnection extends AbstractHttpConnection implements Async finally { setCurrentConnection(null); - _parser.returnBuffers(); - _generator.returnBuffers(); - + if (!_request.isAsyncStarted()) + { + _parser.returnBuffers(); + _generator.returnBuffers(); + } + // Safety net to catch spinning if (some_progress) _total_no_progress=0; diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java index 736a7d64e02..bf932196daf 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/AsyncRequestReadTest.java @@ -62,10 +62,11 @@ public class AsyncRequestReadTest server.stop(); server.join(); } - + @Test public void test() throws Exception { + total=0; final Socket socket = new Socket("localhost",connector.getLocalPort()); byte[] content = new byte[16*4096];