jerqi commented on code in PR #1455:
URL: 
https://github.com/apache/incubator-uniffle/pull/1455#discussion_r1452906206


##########
common/src/main/java/org/apache/uniffle/common/netty/TransportFrameDecoder.java:
##########
@@ -67,9 +67,17 @@ public void channelRead(ChannelHandlerContext ctx, Object 
data) {
       if (frame == null) {
         break;
       }
-      Message msg = Message.decode(curType, frame);
-      if (msg.body() == null) {
-        frame.release();
+      Message msg = null;
+      try {
+        msg = Message.decode(curType, frame);
+      } finally {
+        boolean shouldRelease =
+            msg != null

Review Comment:
   > You mean like this?
   > 
   > ```
   > private static boolean shouldRelease(Message msg) {
   >     boolean shouldRelease =
   >         msg == null
   >             || (msg != null
   >                 && (msg.body() == null
   >                     || (msg.body().byteBuf() != null
   >                         && msg.body().byteBuf().readableBytes() == 0)));
   >     return shouldRelease;
   >   }
   > ```
   > 
   > For testing purposes, maybe private -> public?
   
   You can simplify the code like 
   private static boolean shouldRelease(Message msg) {
      if (msg == null) {
           return true;
       }
       if (mgs.body() == null) {
           return true;
      }
       boolean shouldRelease = (msg.body().byteBuf() != null
                           && msg.body().byteBuf().readableBytes() == 0)));
       return shouldRelease;
     }



-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to