akpatnam25 commented on code in PR #3630:
URL: https://github.com/apache/celeborn/pull/3630#discussion_r2984516000
##########
common/src/main/java/org/apache/celeborn/common/network/protocol/SslMessageEncoder.java:
##########
@@ -89,15 +86,9 @@ public void encode(ChannelHandlerContext ctx, Message in,
List<Object> out) thro
assert header.writableBytes() == 0;
if (body != null && bodyLength > 0) {
- if (body instanceof ByteBuf) {
- out.add(Unpooled.wrappedBuffer(header, (ByteBuf) body));
- } else if (body instanceof InputStream || body instanceof ChunkedStream)
{
- // For now, assume the InputStream is doing proper chunking.
- out.add(new EncryptedMessageWithHeader(in.body(), header, body,
bodyLength));
- } else {
- throw new IllegalArgumentException(
- "Body must be a ByteBuf, ChunkedStream or an InputStream");
- }
+ // We transfer ownership of the reference on in.body() to
EncryptedMessageWithHeader.
+ // This reference will be freed when EncryptedMessageWithHeader.close()
is called.
+ out.add(new EncryptedMessageWithHeader(in.body(), header, body,
bodyLength));
} else {
out.add(header);
}
Review Comment:
@SteNicholas - I don't think this is a bug. This is existing behavior
anyways, so I don't think we should change this. There is no data in the buffer
when bodyLength == 0, so there is no leak. The header anyways gets added in the
else block.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
