Reference-counted SslEngines retain a reference to their parent SslContext (#9626)

Motivation:
With the Netty ref-counted OpenSSL implementation the parent SslContext
maintains state necessary for the SslEngine's it produces. However, it's
possible for the parent context to be closed and release those resources
before the child engines are finished which causes problems.

Modification:
Spawned ReferenceCountedOpenSslEngine's retain a reference to their
parent ReferenceCountedOpenSslContext.

Result:
The lifetime of the shared data is extended to include the lifetime of
the dependents.
This commit is contained in:
Bryce Anderson 2019-10-07 00:12:54 -06:00 committed by Norman Maurer
parent 4ec6db48dd
commit 3f7a7949db
2 changed files with 29 additions and 0 deletions

View File

@ -181,6 +181,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
boolean closed = leak.close(ReferenceCountedOpenSslEngine.this);
assert closed;
}
parentContext.release();
}
};
@ -208,6 +209,7 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
final ByteBufAllocator alloc;
private final OpenSslEngineMap engineMap;
private final OpenSslApplicationProtocolNegotiator apn;
private final ReferenceCountedOpenSslContext parentContext;
private final OpenSslSession session;
private final ByteBuffer[] singleSrcBuffer = new ByteBuffer[1];
private final ByteBuffer[] singleDstBuffer = new ByteBuffer[1];
@ -362,6 +364,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
}
}
// Now that everything looks good and we're going to successfully return the
// object so we need to retain a reference to the parent context.
parentContext = context;
parentContext.retain();
// Only create the leak after everything else was executed and so ensure we don't produce a false-positive for
// the ResourceLeakDetector.
leak = leakDetection ? leakDetector.track(this) : null;

View File

@ -15,12 +15,15 @@
*/
package io.netty.handler.ssl;
import io.netty.buffer.UnpooledByteBufAllocator;
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
import io.netty.util.ReferenceCountUtil;
import org.junit.Test;
import javax.net.ssl.SSLEngine;
import static junit.framework.TestCase.*;
public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest {
public ReferenceCountedOpenSslEngineTest(BufferType type, ProtocolCipherCombo combo, boolean delegate,
@ -77,4 +80,23 @@ public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest {
}
return context;
}
@Test
public void parentContextIsRetainedByChildEngines() throws Exception {
SslContext clientSslCtx = SslContextBuilder.forClient()
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
.build();
SSLEngine engine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
assertEquals(ReferenceCountUtil.refCnt(clientSslCtx), 2);
cleanupClientSslContext(clientSslCtx);
assertEquals(ReferenceCountUtil.refCnt(clientSslCtx), 1);
cleanupClientSslEngine(engine);
assertEquals(ReferenceCountUtil.refCnt(clientSslCtx), 0);
}
}