From 8db8aca1e7bac8a927bc82968291abbb5ade96be 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(); } } }