yzang commented on a change in pull request #913: Refactor FileInfo locking and 
refcounting out IndexPersistenceMgr
URL: https://github.com/apache/bookkeeper/pull/913#discussion_r158849867
 
 

 ##########
 File path: 
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
 ##########
 @@ -354,8 +316,7 @@ private void getActiveLedgers() throws IOException {
      */
     void removeLedger(Long ledgerId) throws IOException {
         // Delete the ledger's index file and close the FileInfo
-        FileInfo fi = null;
-        fileInfoLock.writeLock().lock();
 
 Review comment:
   Remove ledger is not synchronized with getFileInfo, so it will introduce 
race conditions.
   
   Race Condition:
   1. Thread A is on line 221 and get a FileInfo from cache successfully, but 
haven't increment the refCount.
   2. Thread B is on line 335 and after it invalidate the FileInfo from cache, 
refCount will becomes 0 so the underline FileChannel will be closed.
   3. Now Thread A continues to increment the refCount, and returns the closed 
FileInfo to application.
   4. java.nio.channels.ClosedChannelException will be thrown when we starts to 
use this FileInfo.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to