Recursion protection for SSL flush try again #2233

Protecton from recursion in SSL flush try again #2233
This would not be needed if we could make flush iterate when necessary.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2018-03-15 10:41:50 +11:00
parent afc820c7f9
commit c8d6f0a3bb
1 changed files with 31 additions and 6 deletions

View File

@ -81,7 +81,8 @@ import org.eclipse.jetty.util.thread.Invocable;
public class SslConnection extends AbstractConnection public class SslConnection extends AbstractConnection
{ {
private static final Logger LOG = Log.getLogger(SslConnection.class); private static final Logger LOG = Log.getLogger(SslConnection.class);
private static final ThreadLocal<Boolean> __tryWriteAgain = new ThreadLocal<>();
private final List<SslHandshakeListener> handshakeListeners = new ArrayList<>(); private final List<SslHandshakeListener> handshakeListeners = new ArrayList<>();
private final ByteBufferPool _bufferPool; private final ByteBufferPool _bufferPool;
private final SSLEngine _sslEngine; private final SSLEngine _sslEngine;
@ -95,7 +96,7 @@ public class SslConnection extends AbstractConnection
private int _renegotiationLimit = -1; private int _renegotiationLimit = -1;
private boolean _closedOutbound; private boolean _closedOutbound;
private boolean _allowMissingCloseMessage = true; private boolean _allowMissingCloseMessage = true;
private abstract class RunnableTask implements Runnable, Invocable private abstract class RunnableTask implements Runnable, Invocable
{ {
private final String _operation; private final String _operation;
@ -506,11 +507,12 @@ public class SslConnection extends AbstractConnection
} }
else else
{ {
// TODO This should not be required and we should be able to do all retries within flush
// We can get here because the WriteFlusher might not see progress // We can get here because the WriteFlusher might not see progress
// when it has just flushed the encrypted data, but not consumed anymore // when it has just flushed the encrypted data, but not consumed anymore
// of the application buffers. This is mostly avoided by another iteration // of the application buffers. This is mostly avoided by another iteration
// within DecryptedEndPoint flush(), but I cannot convince myself that // within DecryptedEndPoint flush(), but this still occurs sometime on some
// this is never ever the case. // tests on some systems??? More investigation is needed!
try_again = true; try_again = true;
} }
} }
@ -526,12 +528,35 @@ public class SslConnection extends AbstractConnection
{ {
// don't bother writing, just notify of close // don't bother writing, just notify of close
getWriteFlusher().onClose(); getWriteFlusher().onClose();
return;
}
// TODO this ugly recursion protection is only needed until we remove the try again
// logic from this method and make flush do the try again.
Boolean tryWriteAgain = __tryWriteAgain.get();
if (tryWriteAgain==null)
{
try
{
// Keep running complete write until
__tryWriteAgain.set(Boolean.FALSE);
do
{
_runCompleteWrite.run();
}
while (Boolean.TRUE.equals(__tryWriteAgain.get()));
}
finally
{
__tryWriteAgain.remove();
}
} }
else else
{ {
// Try again // Don't recurse but get top caller to iterate
_runCompleteWrite.run(); __tryWriteAgain.set(Boolean.TRUE);
} }
} }
} }