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

 ##########
 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 
   I don't want to change the behavior, I am fine with calling getEntry only 
once. I only wanted to make it more clear.
   When I and my colleague Diego saw actual code we got a little worried and we 
needed a double check with you and Matteo on the mailing list to be sure that 
there was no real problem.
   
   Anyway I am ok with reverting for the getEntry() as the IlegalStateException 
will be thrown in ByteBuf#release().
   
   But for the InputStream case I think it will be too late to see the error at 
the 'close' of the inputstream. I will revert if you prefer, no problem
   
 
----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to