Skip to content
Permalink
Browse files

Avoid potential memory leak in `ByteToMessageDecoder` (#937)

Motivation:

`ByteToMessageDecoder` does not release the received
`ByteBuf` message when it copies data to the cumulation
buffer. It's not an issue at this moment, because we always
see unpooled buffers here (because `CopyByteBufHandler`
copies data to unpooled memory), but we should release
any way to prevent memory leaks if this assumption changes.

Modifications:

- Add `try-finally` block to release inbound `ByteBuf` after
we copied it to the cumulation buffer;

Result:

No memory leaks in `ByteToMessageDecoder`.
  • Loading branch information
idelpivnitskiy committed Feb 14, 2020
1 parent 349f2ca commit 2c4bbac202e3ef324c4de57e455565e6d250ee13
@@ -155,16 +155,21 @@ public void channelRead(ChannelHandlerContext ctx, Object msg) {
if (cumulation == null) {
cumulation = data;
} else {
final int required = data.readableBytes();
if (required > cumulation.maxWritableBytes() ||
(required > cumulation.maxFastWritableBytes() && cumulation.refCnt() > 1)) {
// Expand cumulation (by replacing it) under the following conditions:
// - cumulation cannot be resized to accommodate the additional data
// - cumulation can be expanded with a reallocation operation to accommodate but the buffer is
// assumed to be shared (e.g. refCnt() > 1) and the reallocation may not be safe.
cumulation = swapAndCopyCumulation(ctx.alloc(), cumulation, data);
} else {
cumulation.writeBytes(data);
try {
final int required = data.readableBytes();
if (required > cumulation.maxWritableBytes() ||
(required > cumulation.maxFastWritableBytes() && cumulation.refCnt() > 1)) {
// Expand cumulation (by replacing it) under the following conditions:
// - cumulation cannot be resized to accommodate the additional data
// - cumulation can be expanded with a reallocation operation to accommodate but the buffer
// is assumed to be shared (e.g. refCnt() > 1) and the reallocation may not be safe.
cumulation = swapAndCopyCumulation(ctx.alloc(), cumulation, data);
} else {
cumulation.writeBytes(data);
}
} finally {
// Release data after it was copied to the cumulation buffer:
data.release();
}
}
callDecode(ctxWrapper, cumulation);

0 comments on commit 2c4bbac

Please sign in to comment.
You can’t perform that action at this time.