diff --git a/database/ffldb/blockio.go b/database/ffldb/blockio.go index 2b415a17b0..77f97f592e 100644 --- a/database/ffldb/blockio.go +++ b/database/ffldb/blockio.go @@ -281,17 +281,7 @@ func (s *blockStore) openFile(fileNum uint32) (*lockableFile, error) { lruList := s.openBlocksLRU if lruList.Len() >= maxOpenFiles { lruFileNum := lruList.Remove(lruList.Back()).(uint32) - oldBlockFile := s.openBlockFiles[lruFileNum] - - // Close the old file under the write lock for the file in case - // any readers are currently reading from it so it's not closed - // out from under them. - oldBlockFile.Lock() - _ = oldBlockFile.file.Close() - oldBlockFile.Unlock() - - delete(s.openBlockFiles, lruFileNum) - delete(s.fileNumToLRUElem, lruFileNum) + s.closeFile(lruFileNum) } s.fileNumToLRUElem[fileNum] = lruList.PushFront(fileNum) s.lruMutex.Unlock() @@ -302,11 +292,36 @@ func (s *blockStore) openFile(fileNum uint32) (*lockableFile, error) { return blockFile, nil } +// closeFile checks that the file corresponding to the file number is open and +// if it is, it closes it in a concurrency safe manner and cleans up associated +// data in the blockstore struct. +func (s *blockStore) closeFile(fileNum uint32) { + blockFile := s.openBlockFiles[fileNum] + if blockFile == nil { + return + } + + // Close the old file under the write lock for the file in case + // any readers are currently reading from it so it's not closed + // out from under them. + blockFile.Lock() + _ = blockFile.file.Close() + blockFile.Unlock() + + delete(s.openBlockFiles, fileNum) + delete(s.fileNumToLRUElem, fileNum) +} + // deleteFile removes the block file for the passed flat file number. The file // must already be closed and it is the responsibility of the caller to do any // other state cleanup necessary. func (s *blockStore) deleteFile(fileNum uint32) error { filePath := blockFilePath(s.basePath, fileNum) + blockFile := s.openBlockFiles[fileNum] + if blockFile != nil { + err := fmt.Errorf("attempted to delete open file at %v", filePath) + return makeDbErr(database.ErrDriverSpecific, err.Error(), err) + } if err := os.Remove(filePath); err != nil { return makeDbErr(database.ErrDriverSpecific, err.Error(), err) } diff --git a/database/ffldb/db.go b/database/ffldb/db.go index 3e96bfc738..60103aaa58 100644 --- a/database/ffldb/db.go +++ b/database/ffldb/db.go @@ -1630,6 +1630,9 @@ func (tx *transaction) writePendingAndCommit() error { // We do this first before doing any of the writes as we can't undo // deletions of files. for _, fileNum := range tx.pendingDelFileNums { + // Make sure the file is closed before attempting to delete it. + tx.db.store.closeFile(fileNum) + err := tx.db.store.deleteFileFunc(fileNum) if err != nil { // Nothing we can do if we fail to delete blocks besides diff --git a/database/ffldb/driver_test.go b/database/ffldb/driver_test.go index 0b2f452032..e48491a12d 100644 --- a/database/ffldb/driver_test.go +++ b/database/ffldb/driver_test.go @@ -335,6 +335,20 @@ func TestPrune(t *testing.T) { t.Fatal(err) } + // Open the first block file before the pruning happens in the + // code snippet below. This let's us test that block files are + // properly closed before attempting to delete them. + err = db.View(func(tx database.Tx) error { + _, err := tx.FetchBlock(blocks[0].Hash()) + if err != nil { + return err + } + return nil + }) + if err != nil { + t.Fatal(err) + } + var deletedBlocks []chainhash.Hash // This should leave 3 files on disk.