andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
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 GitHub
parent fd47554669
commit 36eb399b4b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 23 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
handler/src/main/java/io/netty/handler/ssl/SslHandler.java
View File

@ -37,7 +37,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;
@ -674,32 +673,32 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
@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);
} }
} }