From afe489e986faa54a0a102e6320b2242d9aa4db2e Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Fri, 10 Aug 2012 15:21:52 +1000 Subject: [PATCH] jetty-9 potential SSL fix --- .../eclipse/jetty/io/AbstractConnection.java | 1 + .../org/eclipse/jetty/io/SelectorManager.java | 20 +-------- .../eclipse/jetty/io/ssl/SslConnection.java | 45 ++++++++++++++----- .../eclipse/jetty/io/SslConnectionTest.java | 15 +++++-- .../org/eclipse/jetty/spdy/AbstractTest.java | 5 ++- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java index 633422234af..461b1c3ff85 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/AbstractConnection.java @@ -92,6 +92,7 @@ public abstract class AbstractConnection implements Connection */ public void fillInterested() { + LOG.debug("fillInterested {}",this); if (_readInterested.compareAndSet(false, true)) getEndPoint().fillInterested(null, _readCallback); } diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java index fa4157981e8..dc33ff8fe0a 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java @@ -154,7 +154,6 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa */ protected void endPointOpened(EndPoint endpoint) { - // TODO should this be dispatched endpoint.onOpen(); } @@ -165,7 +164,6 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa */ protected void endPointClosed(EndPoint endpoint) { - // TODO should this be dispatched endpoint.onClose(); } @@ -176,14 +174,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa */ public void connectionOpened(final Connection connection) { - execute(new Runnable() - { - @Override - public void run() - { - connection.onOpen(); - } - }); + connection.onOpen(); } /** @@ -193,14 +184,7 @@ public abstract class SelectorManager extends AbstractLifeCycle implements Dumpa */ public void connectionClosed(final Connection connection) { - execute(new Runnable() - { - @Override - public void run() - { - connection.onClose(); - } - }); + connection.onClose(); } /** diff --git a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 6a13c6123dd..2ff9427d189 100644 --- a/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -90,6 +90,15 @@ public class SslConnection extends AbstractConnection } }; + private final Runnable _runWriteEmpty = new Runnable() + { + @Override + public void run() + { + _decryptedEndPoint.write(null, new Callback.Empty<>(), BufferUtil.EMPTY_BUFFER); + } + }; + public SslConnection(ByteBufferPool byteBufferPool, Executor executor, EndPoint endPoint, SSLEngine sslEngine) { super(endPoint, executor, true); @@ -119,7 +128,7 @@ public class SslConnection extends AbstractConnection _sslEngine.beginHandshake(); if (_sslEngine.getUseClientMode()) - _decryptedEndPoint.write(null, new Callback.Empty<>(), BufferUtil.EMPTY_BUFFER); + getExecutor().execute(_runWriteEmpty); } catch (SSLException x) { @@ -138,23 +147,24 @@ public class SslConnection extends AbstractConnection // to do the fill and/or flush again and these calls will do the actually // filling. - LOG.debug("{} onFillable", this); + LOG.debug("onFillable {}", getEndPoint()); - synchronized(_decryptedEndPoint) + // wake up whoever is doing the fill or the flush so they can + // do all the filling, unwrapping ,wrapping and flushing + _decryptedEndPoint.getFillInterest().fillable(); + + + // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read + synchronized (_decryptedEndPoint) { - // wake up whoever is doing the fill or the flush so they can - // do all the filling, unwrapping ,wrapping and flushing - _decryptedEndPoint.getFillInterest().fillable(); - - // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read if (_decryptedEndPoint._flushRequiresFillToProgress) { _decryptedEndPoint._flushRequiresFillToProgress = false; - getExecutor().execute(_runCompletWrite); } } - LOG.debug("{} onFilled", this); + + LOG.debug("onFilled {}", getEndPoint()); } /* ------------------------------------------------------------ */ @@ -282,6 +292,7 @@ public class SslConnection extends AbstractConnection // if neither than we should just try the flush again. synchronized (DecryptedEndPoint.this) { + LOG.debug("onIncompleteFlush {}",getEndPoint()); // If we have pending output data, if (BufferUtil.hasContent(_encryptedOutput)) { @@ -290,8 +301,11 @@ public class SslConnection extends AbstractConnection getEndPoint().write(null, _writeCallback, _encryptedOutput); } else if (_sslEngine.getHandshakeStatus() == HandshakeStatus.NEED_UNWRAP) + { // we are actually read blocked in order to write + _flushRequiresFillToProgress=true; SslConnection.this.fillInterested(); + } else // try the flush again getWriteFlusher().completeWrite(); @@ -522,6 +536,13 @@ public class SslConnection extends AbstractConnection } finally { + // If we are handshaking, then wake up any waiting write as well as it may have been blocked on the read + if (_decryptedEndPoint._flushRequiresFillToProgress) + { + _decryptedEndPoint._flushRequiresFillToProgress = false; + getExecutor().execute(_runCompletWrite); + } + if (_encryptedInput != null && !_encryptedInput.hasRemaining()) { _bufferPool.release(_encryptedInput); @@ -551,6 +572,7 @@ public class SslConnection extends AbstractConnection // or better yet by using EndPoint#write to do the flushing. LOG.debug("{} flush enter {}", SslConnection.this, Arrays.toString(appOuts)); + int consumed=0; try { if (_cannotAcceptMoreAppDataToFlush) @@ -560,7 +582,6 @@ public class SslConnection extends AbstractConnection if (_encryptedOutput == null) _encryptedOutput = _bufferPool.acquire(_sslEngine.getSession().getPacketBufferSize() * 2, _encryptedDirectBuffers); - int consumed=0; while (true) { // do the funky SSL thang! @@ -648,7 +669,7 @@ public class SslConnection extends AbstractConnection } finally { - LOG.debug("{} flush exit", SslConnection.this); + LOG.debug("{} flush exit {}", SslConnection.this,consumed); releaseNetOut(); } } diff --git a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java index 071a54edd68..a10eefa3bac 100644 --- a/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java +++ b/jetty-io/src/test/java/org/eclipse/jetty/io/SslConnectionTest.java @@ -126,7 +126,17 @@ public class SslConnectionTest if (_testFill) fillInterested(); else - getEndPoint().write(null,_writeCallback,BufferUtil.toBuffer("Hello Client")); + { + getExecutor().execute(new Runnable() + { + + @Override + public void run() + { + getEndPoint().write(null,_writeCallback,BufferUtil.toBuffer("Hello Client")); + } + }); + } } @Override @@ -215,14 +225,13 @@ public class SslConnectionTest @Test public void testWriteOnConnect() throws Exception { - //Log.getRootLogger().setDebugEnabled(true); _testFill=false; for (int i=0;i<1;i++) { _writeCallback = new FutureCallback<>(); Socket client = newClient(); - client.setSoTimeout(60000); + client.setSoTimeout(600000); // TODO reduce after debugging SocketChannel server = _connector.accept(); server.configureBlocking(false); diff --git a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java index 8822d1391f0..ba65761649e 100644 --- a/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java +++ b/jetty-spdy/spdy-jetty/src/test/java/org/eclipse/jetty/spdy/AbstractTest.java @@ -56,8 +56,9 @@ public abstract class AbstractTest protected InetSocketAddress startServer(short version, ServerSessionFrameListener listener) throws Exception { - server = new Server(); - ((QueuedThreadPool)server.getThreadPool()).setName("server"); + QueuedThreadPool pool = new QueuedThreadPool(); + pool.setName(pool.getName()+"-server"); + server = new Server(pool); if (connector == null) connector = newSPDYServerConnector(listener); if (listener == null)