[GitHub] eolivelli commented on a change in pull request #276: Issue-271 LedgerHandle#readEntries leaks ByteBufs

[gi...@git.apache.org]

eolivelli commented on a change in pull request #276: Issue-271 LedgerHandle#readEntries leaks ByteBufs
URL: https://github.com/apache/bookkeeper/pull/276#discussion_r129248110
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerEntry.java
 ##########
 @@ -53,22 +54,49 @@ public long getLength() {
         return length;
     }
 
+    /**
+     * Returns the content of the entry.
+     * This method can be called only once.
+     * While using v2 wire protocol this method will automatically release the internal ByteBuf
+     * @return
+     */
     public byte[] getEntry() {
+        if (data == null) {
 
 Review comment:
   @sijie the first time getEntry is called we will set data to null, this way the client cannot re-use the buffer anymore.
   On an eventual "second call" we will throw an IllegalStateException which explains the error.
   if I drop the 'if' the user will get a NullPointerException, but this is not very clear and IMHO it is bad practice.
   
   Unfortunately the ByteBuf can be used only once, because we have no clue to reset the original start position for the entry
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services