From 60cc0db3952a95746800596923f628f8d23f4bd3 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Mon, 20 Jul 2009 08:28:52 +0000 Subject: [PATCH] JETTY-1068 Avoid busy flush of async SSL git-svn-id: svn+ssh://dev.eclipse.org/svnroot/rt/org.eclipse.jetty/jetty/trunk@565 7e9141cc-0065-0410-87d8-b60c137991c4 --- VERSION.txt | 1 + .../http/ssl/SslSelectChannelEndPoint.java | 139 +++++++----------- .../server/ssl/SslSelectChannelConnector.java | 4 - 3 files changed, 51 insertions(+), 93 deletions(-) diff --git a/VERSION.txt b/VERSION.txt index adf852a4090..fee83e2715a 100644 --- a/VERSION.txt +++ b/VERSION.txt @@ -3,6 +3,7 @@ jetty-7.0.0.RC2-SNAPSHOT jetty-7.0.0.RC1 15 June 2009 + JETTY-1066 283357 400 response for bad URIs + + JETTY-1068 Avoid busy flush of async SSL + 283344 Startup on windows is broken jetty-7.0.0.RC0 8 June 2009 diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/ssl/SslSelectChannelEndPoint.java b/jetty-http/src/main/java/org/eclipse/jetty/http/ssl/SslSelectChannelEndPoint.java index 532b3cf48e4..2a7d5121c43 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/ssl/SslSelectChannelEndPoint.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/ssl/SslSelectChannelEndPoint.java @@ -62,25 +62,10 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint private boolean _closing=false; private SSLEngineResult _result; - private String _last; private final boolean _debug = __log.isDebugEnabled(); // snapshot debug status for optimizer - // TODO get rid of this - // StringBuilder h = new StringBuilder(500); - /* - class H - { - H append(Object o) - { - System.err.print(o); - return this; - } - }; - H h = new H(); - */ - /* ------------------------------------------------------------ */ public SslSelectChannelEndPoint(Buffers buffers,SocketChannel channel, SelectorManager.SelectSet selectSet, SelectionKey key, SSLEngine engine) throws IOException @@ -98,7 +83,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _inNIOBuffer=(NIOBuffer)_buffers.getBuffer(_session.getPacketBufferSize()); _inBuffer=_inNIOBuffer.getByteBuffer(); - // h.append("CONSTRUCTED\n"); if (_debug) __log.debug(_session+" channel="+channel); } @@ -135,7 +119,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint /* ------------------------------------------------------------ */ protected void doIdleExpired() { - // h.append("IDLE EXPIRED\n"); super.idleExpired(); } @@ -144,33 +127,31 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint { // TODO - this really should not be done in a loop here - but with async callbacks. - // h.append("CLOSE\n"); _closing=true; try { - int tries=0; - - while (_outNIOBuffer.length()>0) + if (isBufferingOutput()) { - // TODO REMOVE loop check - if (tries++>100) - throw new IllegalStateException(); flush(); - Thread.sleep(100); // TODO yuck + while (isOpen() && isBufferingOutput()) + { + Thread.sleep(100); // TODO non blocking + flush(); + } } _engine.closeOutbound(); loop: while (isOpen() && !(_engine.isInboundDone() && _engine.isOutboundDone())) { - // TODO REMOVE loop check - if (tries++>100) - throw new IllegalStateException(); - - if (_outNIOBuffer.length()>0) + if (isBufferingOutput()) { flush(); - Thread.sleep(100); // TODO yuck + while (isOpen() && isBufferingOutput()) + { + Thread.sleep(100); // TODO non blocking + flush(); + } } if (_debug) __log.debug(_session+" closing "+_engine.getHandshakeStatus()); @@ -187,7 +168,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint ByteBuffer bbuffer = ((NIOBuffer)buffer).getByteBuffer(); if (!unwrap(bbuffer) && _engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP) { - // h.append("break loop\n"); break loop; } } @@ -213,16 +193,12 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint case NEED_WRAP: { - if (_outNIOBuffer.length()>0) - flush(); - try { _outNIOBuffer.compact(); int put=_outNIOBuffer.putIndex(); _outBuffer.position(put); _result=null; - _last="close wrap"; _result=_engine.wrap(__NO_BUFFERS,_outBuffer); if (_debug) __log.debug(_session+" close wrap "+_result); _outNIOBuffer.setPutIndex(put+_result.bytesProduced()); @@ -232,8 +208,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _outBuffer.position(0); } - flush(); - break; } } @@ -277,19 +251,16 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint { unwrap(bbuf); - int tries=0, wraps=0; + int wraps=0; loop: while (true) { - // TODO REMOVE loop check - if (tries++>100) - throw new IllegalStateException(); - - // h.append("Fill(Buffer)\n"); - - if (_outNIOBuffer.length()>0) + if (isBufferingOutput()) + { flush(); + if (isBufferingOutput()) + break loop; + } - // h.append("status=").append(_engine.getHandshakeStatus()).append('\n'); switch(_engine.getHandshakeStatus()) { case FINISHED: @@ -301,7 +272,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint case NEED_UNWRAP: if (!unwrap(bbuf) && _engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP) { - // h.append("break loop\n"); break loop; } break; @@ -311,11 +281,11 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint Runnable task; while ((task=_engine.getDelegatedTask())!=null) { - // h.append("run task\n"); task.run(); } + if(initialStatus==HandshakeStatus.NOT_HANDSHAKING && - HandshakeStatus.NEED_UNWRAP==_engine.getHandshakeStatus() && wraps==0) + _engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP && wraps==0) { // This should be NEED_WRAP // The fix simply detects the signature of the bug and then close the connection (fail-fast) so that ff3 will delegate to using SSL instead of TLS. @@ -338,7 +308,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint int put=_outNIOBuffer.putIndex(); _outBuffer.position(); _result=null; - _last="fill wrap"; _result=_engine.wrap(__NO_BUFFERS,_outBuffer); if (_debug) __log.debug(_session+" fill wrap "+_result); switch(_result.getStatus()) @@ -350,7 +319,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _closing=true; } - // h.append("wrap ").append(_result).append('\n'); _outNIOBuffer.setPutIndex(put+_result.bytesProduced()); } finally @@ -401,17 +369,13 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint int tries=0; loop: while (true) - { - // TODO REMOVE loop check - if (tries++>100) - throw new IllegalStateException(); - - // h.append("Flush ").append(tries).append(' ').append(_outNIOBuffer.length()).append('\n'); - + { if (_outNIOBuffer.length()>0) + { flush(); - - // h.append(_engine.getHandshakeStatus()).append('\n'); + if (isBufferingOutput()) + break loop; + } switch(_engine.getHandshakeStatus()) { @@ -425,7 +389,17 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint break loop; } - int c=(header!=null && buffer!=null)?wrap(header,buffer):wrap(header); + int c; + if (header!=null && header.length()>0) + { + if (buffer!=null && buffer.length()>0) + c=wrap(header,buffer); + else + c=wrap(header); + } + else + c=wrap(buffer); + if (c>0) { consumed+=c; @@ -447,7 +421,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint ByteBuffer bbuf = ((NIOBuffer)buf).getByteBuffer(); if (!unwrap(bbuf) && _engine.getHandshakeStatus()==HandshakeStatus.NEED_UNWRAP) { - // h.append("break").append('\n'); break loop; } } @@ -463,7 +436,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint Runnable task; while ((task=_engine.getDelegatedTask())!=null) { - // h.append("run task\n"); task.run(); } break; @@ -479,7 +451,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint int put=_outNIOBuffer.putIndex(); _outBuffer.position(); _result=null; - _last="flush wrap"; _result=_engine.wrap(__NO_BUFFERS,_outBuffer); if (_debug) __log.debug(_session+" flush wrap "+_result); switch(_result.getStatus()) @@ -490,7 +461,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint case CLOSED: _closing=true; } - // h.append("wrap=").append(_result).append('\n'); _outNIOBuffer.setPutIndex(put+_result.bytesProduced()); } finally @@ -500,6 +470,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint } flush(); + if (isBufferingOutput()) + break loop; break; } @@ -512,18 +484,16 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint /* ------------------------------------------------------------ */ public void flush() throws IOException { - while (_outNIOBuffer.length()>0) + int len=_outNIOBuffer.length(); + if (isBufferingOutput()) { int flushed=super.flush(_outNIOBuffer); - if (_debug) __log.debug(_session+" flushed "+flushed); - - // h.append("flushed=").append(flushed).append(" of ").append(_outNIOBuffer.length()).append('\n'); - if (flushed==0) + if (_debug) __log.debug(_session+" Flushed "+flushed+"/"+len); + if (isBufferingOutput()) { Thread.yield(); - //noinspection UnusedAssignment flushed=super.flush(_outNIOBuffer); - // h.append("flushed2=").append(flushed).append(" of ").append(_outNIOBuffer.length()).append('\n'); + if (_debug) __log.debug(_session+" flushed "+flushed+"/"+len); } } } @@ -556,7 +526,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint { int filled=super.fill(_inNIOBuffer); if (_debug) __log.debug(_session+" unwrap filled "+filled); - // h.append("fill=").append(filled).append('\n'); if (filled<=0) break; total_filled+=filled; @@ -568,8 +537,6 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint break; } } - - // h.append("inNIOBuffer=").append(_inNIOBuffer.length()).append('\n'); if (_inNIOBuffer.length()==0) { @@ -583,10 +550,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _inBuffer.position(_inNIOBuffer.getIndex()); _inBuffer.limit(_inNIOBuffer.putIndex()); _result=null; - _last="unwrap"; _result=_engine.unwrap(_inBuffer,buffer); if (_debug) __log.debug(_session+" unwrap unwrap "+_result); - // h.append("unwrap=").append(_result).append('\n'); _inNIOBuffer.skip(_result.bytesConsumed()); } finally @@ -658,10 +623,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _outBuffer.limit(_outBuffer.capacity()); _result=null; - _last="wrap wrap"; _result=_engine.wrap(_gather,_outBuffer); if (_debug) __log.debug(_session+" wrap wrap "+_result); - // h.append("wrap2=").append(_result).append('\n'); _outNIOBuffer.setGetIndex(0); _outNIOBuffer.setPutIndex(_result.bytesProduced()); consumed=_result.bytesConsumed(); @@ -712,13 +675,13 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint } /* ------------------------------------------------------------ */ - private int wrap(final Buffer header) throws IOException + private int wrap(final Buffer buffer) throws IOException { - _gather[0]=extractOutputBuffer(header,0); + _gather[0]=extractOutputBuffer(buffer,0); synchronized(_gather[0]) { - _gather[0].position(header.getIndex()); - _gather[0].limit(header.putIndex()); + _gather[0].position(buffer.getIndex()); + _gather[0].limit(buffer.putIndex()); int consumed=0; synchronized(_outBuffer) @@ -729,10 +692,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint _outBuffer.position(0); _outBuffer.limit(_outBuffer.capacity()); _result=null; - _last="wrap wrap"; _result=_engine.wrap(_gather[0],_outBuffer); if (_debug) __log.debug(_session+" wrap wrap "+_result); - // h.append("wrap1=").append(_result).append('\n'); _outNIOBuffer.setGetIndex(0); _outNIOBuffer.setPutIndex(_result.bytesProduced()); consumed=_result.bytesConsumed(); @@ -743,8 +704,8 @@ public class SslSelectChannelEndPoint extends SelectChannelEndPoint if (consumed>0) { - int len=consumed