From 7150b42a56c9a255e9794df74ad0568e0cce32ce Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 23 Oct 2019 05:28:42 -0700 Subject: [PATCH] =?UTF-8?q?Refactor=20SslHandler=20internals=20to=20always?= =?UTF-8?q?=20use=20heap=20buffers=20for=20JDK=20SSLE=E2=80=A6=20(#9696)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Motivation: We should aim to always use heap buffers when using the JDK SSLEngine for now as it wants to operate on byte[] and so will do internal memory copies if a non heap buffer is used. Beside this it will always return BUFFER_OVERFLOW when a smaller buffer then 16kb is used when calling wrap(...) (even if a very small amount of bytes should be encrypted). This can lead to excercive direct memory usage and pressure for no good reason. Modifications: Refactor internals of SslHandler to ensure we use heap buffers for the JDK SSLEngine impelementation Result: Less direct memory usage when JDK SSLEngine implementation is used --- .../java/io/netty/handler/ssl/SslHandler.java | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 2e9616e187..f1fd5d0505 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -210,14 +210,10 @@ public class SslHandler extends ByteToMessageDecoder { } @Override - int getPacketBufferSize(SslHandler handler) { - return ((ReferenceCountedOpenSslEngine) handler.engine).maxEncryptedPacketLength0(); - } - - @Override - int calculateWrapBufferCapacity(SslHandler handler, int pendingBytes, int numComponents) { - return ((ReferenceCountedOpenSslEngine) handler.engine).calculateMaxLengthForWrap(pendingBytes, - numComponents); + ByteBuf allocateWrapBuffer(SslHandler handler, ByteBufAllocator allocator, + int pendingBytes, int numComponents) { + return allocator.directBuffer(((ReferenceCountedOpenSslEngine) handler.engine) + .calculateMaxLengthForWrap(pendingBytes, numComponents)); } @Override @@ -259,8 +255,10 @@ public class SslHandler extends ByteToMessageDecoder { } @Override - int calculateWrapBufferCapacity(SslHandler handler, int pendingBytes, int numComponents) { - return ((ConscryptAlpnSslEngine) handler.engine).calculateOutNetBufSize(pendingBytes, numComponents); + ByteBuf allocateWrapBuffer(SslHandler handler, ByteBufAllocator allocator, + int pendingBytes, int numComponents) { + return allocator.directBuffer( + ((ConscryptAlpnSslEngine) handler.engine).calculateOutNetBufSize(pendingBytes, numComponents)); } @Override @@ -302,8 +300,15 @@ public class SslHandler extends ByteToMessageDecoder { } @Override - int calculateWrapBufferCapacity(SslHandler handler, int pendingBytes, int numComponents) { - return handler.engine.getSession().getPacketBufferSize(); + ByteBuf allocateWrapBuffer(SslHandler handler, ByteBufAllocator allocator, + int pendingBytes, int numComponents) { + // As for the JDK SSLEngine we always need to allocate buffers of the size required by the SSLEngine + // (normally ~16KB). This is required even if the amount of data to encrypt is very small. Use heap + // buffers to reduce the native memory usage. + // + // Beside this the JDK SSLEngine also (as of today) will do an extra heap to direct buffer copy + // if a direct buffer is used as its internals operate on byte[]. + return allocator.heapBuffer(handler.engine.getSession().getPacketBufferSize()); } @Override @@ -327,23 +332,21 @@ public class SslHandler extends ByteToMessageDecoder { this.cumulator = cumulator; } - int getPacketBufferSize(SslHandler handler) { - return handler.engine.getSession().getPacketBufferSize(); - } - abstract SSLEngineResult unwrap(SslHandler handler, ByteBuf in, int readerIndex, int len, ByteBuf out) throws SSLException; - abstract int calculateWrapBufferCapacity(SslHandler handler, int pendingBytes, int numComponents); - abstract int calculatePendingData(SslHandler handler, int guess); abstract boolean jdkCompatibilityMode(SSLEngine engine); + abstract ByteBuf allocateWrapBuffer(SslHandler handler, ByteBufAllocator allocator, + int pendingBytes, int numComponents); + // BEGIN Platform-dependent flags /** - * {@code true} if and only if {@link SSLEngine} expects a direct buffer. + * {@code true} if and only if {@link SSLEngine} expects a direct buffer and so if a heap buffer + * is given will make an extra memory copy. */ final boolean wantsDirectBuffer; @@ -2103,7 +2106,7 @@ public class SslHandler extends ByteToMessageDecoder { * the specified amount of pending bytes. */ private ByteBuf allocateOutNetBuf(ChannelHandlerContext ctx, int pendingBytes, int numComponents) { - return allocate(ctx, engineType.calculateWrapBufferCapacity(this, pendingBytes, numComponents)); + return engineType.allocateWrapBuffer(this, ctx.alloc(), pendingBytes, numComponents); } /**