Always release the sslEngine inside SslHandler's handlerRemoved0 (#11605)

Motivation:
Make SslHandler's handlerRemoved0 method release the sslEngine
even if it fails in the middle.
See details in https://github.com/netty/netty/issues/11595.

Modifications:
Wrap the release of sslEngine into a finally block.

Result:
The sslEngine would be released eventually.

Co-authored-by: Chen Liu <cliu@splunk.com>
This commit is contained in:
Chen Liu 2021-08-20 09:50:19 -07:00 committed by Norman Maurer
parent fb1face924
commit ea5bc27c83

View File

@ -34,7 +34,6 @@ import io.netty.handler.codec.ByteToMessageDecoder;
import io.netty.handler.codec.DecoderException; import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.UnsupportedMessageTypeException; import io.netty.handler.codec.UnsupportedMessageTypeException;
import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCountUtil;
import io.netty.util.ReferenceCounted;
import io.netty.util.concurrent.DefaultPromise; import io.netty.util.concurrent.DefaultPromise;
import io.netty.util.concurrent.EventExecutor; import io.netty.util.concurrent.EventExecutor;
import io.netty.util.concurrent.Future; import io.netty.util.concurrent.Future;
@ -650,32 +649,32 @@ public class SslHandler extends ByteToMessageDecoder {
@Override @Override
public void handlerRemoved0(ChannelHandlerContext ctx) throws Exception { public void handlerRemoved0(ChannelHandlerContext ctx) throws Exception {
if (!pendingUnencryptedWrites.isEmpty()) { try {
// Check if queue is not empty first because create a new ChannelException is expensive if (!pendingUnencryptedWrites.isEmpty()) {
pendingUnencryptedWrites.releaseAndFailAll(ctx, // Check if queue is not empty first because create a new ChannelException is expensive
new ChannelException("Pending write on removal of SslHandler")); pendingUnencryptedWrites.releaseAndFailAll(ctx,
} new ChannelException("Pending write on removal of SslHandler"));
pendingUnencryptedWrites = null;
SSLHandshakeException cause = null;
// If the handshake is not done yet we should fail the handshake promise and notify the rest of the
// pipeline.
if (!handshakePromise.isDone()) {
cause = new SSLHandshakeException("SslHandler removed before handshake completed");
if (handshakePromise.tryFailure(cause)) {
ctx.fireUserEventTriggered(new SslHandshakeCompletionEvent(cause));
} }
} pendingUnencryptedWrites = null;
if (!sslClosePromise.isDone()) {
if (cause == null) { SSLHandshakeException cause = null;
// If the handshake is not done yet we should fail the handshake promise and notify the rest of the
// pipeline.
if (!handshakePromise.isDone()) {
cause = new SSLHandshakeException("SslHandler removed before handshake completed"); cause = new SSLHandshakeException("SslHandler removed before handshake completed");
if (handshakePromise.tryFailure(cause)) {
ctx.fireUserEventTriggered(new SslHandshakeCompletionEvent(cause));
}
} }
notifyClosePromise(cause); if (!sslClosePromise.isDone()) {
} if (cause == null) {
cause = new SSLHandshakeException("SslHandler removed before handshake completed");
if (engine instanceof ReferenceCounted) { }
((ReferenceCounted) engine).release(); notifyClosePromise(cause);
}
} finally {
ReferenceCountUtil.release(engine);
} }
} }