[GitHub] [bookkeeper] wenbingshen commented on a diff in pull request #3746: Fix memory leak when read only

[GitBox <gi...@apache.org>]

wenbingshen commented on code in PR #3746:
URL: https://github.com/apache/bookkeeper/pull/3746#discussion_r1073074566


##########
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PacketProcessorBase.java:
##########
@@ -175,7 +175,9 @@ public void run() {
             if (request instanceof BookieProtocol.ReadRequest) {
                 requestProcessor.onReadRequestFinish();
             }
-            if (request instanceof BookieProtocol.AddRequest) {
+            if (request instanceof BookieProtocol.ParsedAddRequest) {
+                ((BookieProtocol.ParsedAddRequest) request).release();
+                request.recycle();
                 requestProcessor.onAddRequestFinish();

Review Comment:
   Can the release of ParsedAddRequest be executed in the recycle method? It seems that these two methods are always used together, and there is a forced switch here, which seems a bit awkward.
   
   You can look at org.apache.bookkeeper.proto.BookieProtocol.AddRequest#recycle method
   ```java
           @Override
           public void recycle() {
               ledgerId = -1;
               entryId = -1;
               masterKey = null;
               ReferenceCountUtil.safeRelease(data);
               data = null;
               recyclerHandle.recycle(this);
           }
   ```



-- 
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: commits-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org