From 06ea226a2827a5b31b6a55efac44c8a3b1ed1e2c Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Mon, 27 Oct 2014 13:47:54 -0400 Subject: [PATCH] SslHandler wrap memory leak Motivation: The SslHandler wrap method requires that a direct buffer be passed to the SSLEngine.wrap() call. If the ByteBuf parameter does not have an underlying direct buffer then one is allocated in this method, but it is not released. Modifications: - Release the direct ByteBuffer only accessible in the scope of SslHandler.wrap Result: Memory leak in SslHandler.wrap is fixed. --- .../java/io/netty/handler/ssl/SslHandler.java | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 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 40ef0ce563..9ea071d529 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -436,6 +436,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH private void wrap(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException { ByteBuf out = null; ChannelPromise promise = null; + ByteBufAllocator alloc = ctx.alloc(); try { for (;;) { Object msg = pendingUnencryptedWrites.current(); @@ -453,7 +454,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH out = allocateOutNetBuf(ctx, buf.readableBytes()); } - SSLEngineResult result = wrap(engine, buf, out); + SSLEngineResult result = wrap(alloc, engine, buf, out); if (!buf.isReadable()) { promise = pendingUnencryptedWrites.remove(); @@ -519,12 +520,13 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH private void wrapNonAppData(ChannelHandlerContext ctx, boolean inUnwrap) throws SSLException { ByteBuf out = null; + ByteBufAllocator alloc = ctx.alloc(); try { for (;;) { if (out == null) { out = allocateOutNetBuf(ctx, 0); } - SSLEngineResult result = wrap(engine, Unpooled.EMPTY_BUFFER, out); + SSLEngineResult result = wrap(alloc, engine, Unpooled.EMPTY_BUFFER, out); if (result.bytesProduced() > 0) { ctx.write(out); @@ -574,26 +576,37 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH } } - private SSLEngineResult wrap(SSLEngine engine, ByteBuf in, ByteBuf out) throws SSLException { - ByteBuffer in0 = in.nioBuffer(); - if (!in0.isDirect()) { - ByteBuffer newIn0 = ByteBuffer.allocateDirect(in0.remaining()); - newIn0.put(in0).flip(); - in0 = newIn0; - } + private SSLEngineResult wrap(ByteBufAllocator alloc, SSLEngine engine, ByteBuf in, ByteBuf out) + throws SSLException { + ByteBuf newDirectIn = null; + try { + final ByteBuffer in0; + if (in.isDirect()) { + in0 = in.nioBuffer(); + } else { + int readableBytes = in.readableBytes(); + newDirectIn = alloc.directBuffer(readableBytes); + newDirectIn.writeBytes(in, in.readerIndex(), readableBytes); + in0 = newDirectIn.internalNioBuffer(0, readableBytes); + } - for (;;) { - ByteBuffer out0 = out.nioBuffer(out.writerIndex(), out.writableBytes()); - SSLEngineResult result = engine.wrap(in0, out0); - in.skipBytes(result.bytesConsumed()); - out.writerIndex(out.writerIndex() + result.bytesProduced()); + for (;;) { + ByteBuffer out0 = out.nioBuffer(out.writerIndex(), out.writableBytes()); + SSLEngineResult result = engine.wrap(in0, out0); + in.skipBytes(result.bytesConsumed()); + out.writerIndex(out.writerIndex() + result.bytesProduced()); - switch (result.getStatus()) { + switch (result.getStatus()) { case BUFFER_OVERFLOW: out.ensureWritable(maxPacketBufferSize); break; default: return result; + } + } + } finally { + if (newDirectIn != null) { + newDirectIn.release(); } } }