Ensure we only add OpenSslEngine to the OpenSslEngineMap when handshake is started

Motivation:

We need to ensure we only add the OpenSslEngine to the OpenSslEngineMap when the handshake is started as otherwise we may produce a memory leak when the OpenSslEngine is created but not actually used. This can for example happen if we encounter a connection refused from the remote peer. In this case we will never remove the OpenSslEngine from the OpenSslEngineMap and so it will never be collected (as we hold a reference). This has as affect that the finalizer will never be run as well.

Modifications:

- Lazy add the OpenSslEngine to the OpenSslEngineMap to elimate possible leak.
- Call OpenSslEngine.shutdown() when SslHandler is removed from the ChannelPipeline to free memory asap in all cases.

Result:

No more memory leak with OpenSslEngine if connection is refused.
This commit is contained in:
Norman Maurer 2016-01-04 21:22:48 +01:00
parent 76ef2aaa8c
commit c864c4135c
3 changed files with 9 additions and 4 deletions

View File

@ -295,10 +295,8 @@ public abstract class OpenSslContext extends SslContext {
@Override @Override
public final SSLEngine newEngine(ByteBufAllocator alloc, String peerHost, int peerPort) { public final SSLEngine newEngine(ByteBufAllocator alloc, String peerHost, int peerPort) {
final OpenSslEngine engine = new OpenSslEngine(ctx, alloc, isClient(), sessionContext(), apn, engineMap, return new OpenSslEngine(ctx, alloc, isClient(), sessionContext(), apn, engineMap,
rejectRemoteInitiatedRenegotiation, peerHost, peerPort, keyCertChain, clientAuth); rejectRemoteInitiatedRenegotiation, peerHost, peerPort, keyCertChain, clientAuth);
engineMap.add(engine);
return engine;
} }
/** /**

View File

@ -168,7 +168,7 @@ public final class OpenSslEngine extends SSLEngine {
private final OpenSslApplicationProtocolNegotiator apn; private final OpenSslApplicationProtocolNegotiator apn;
private final boolean rejectRemoteInitiatedRenegation; private final boolean rejectRemoteInitiatedRenegation;
private final OpenSslSession session; private final OpenSslSession session;
private final java.security.cert.Certificate[] localCerts; private final Certificate[] localCerts;
private final ByteBuffer[] singleSrcBuffer = new ByteBuffer[1]; private final ByteBuffer[] singleSrcBuffer = new ByteBuffer[1];
private final ByteBuffer[] singleDstBuffer = new ByteBuffer[1]; private final ByteBuffer[] singleDstBuffer = new ByteBuffer[1];
@ -1185,6 +1185,9 @@ public final class OpenSslEngine extends SSLEngine {
throw exception; throw exception;
} }
// Adding the OpenSslEngine to the OpenSslEngineMap so it can be used in the AbstractCertificateVerifier.
engineMap.add(this);
int code = SSL.doHandshake(ssl); int code = SSL.doHandshake(ssl);
if (code <= 0) { if (code <= 0) {
// Check if we have a pending exception that was created during the handshake and if so throw it after // Check if we have a pending exception that was created during the handshake and if so throw it after

View File

@ -429,6 +429,10 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
// Check if queue is not empty first because create a new ChannelException is expensive // Check if queue is not empty first because create a new ChannelException is expensive
pendingUnencryptedWrites.removeAndFailAll(new ChannelException("Pending write on removal of SslHandler")); pendingUnencryptedWrites.removeAndFailAll(new ChannelException("Pending write on removal of SslHandler"));
} }
if (engine instanceof OpenSslEngine) {
// Call shutdown so we ensure all the native memory is released asap
((OpenSslEngine) engine).shutdown();
}
} }
@Override @Override