Merge pull request #2208 from kcalvinalvin/2024-07-01-close-blockfiles

ffldb: close block files

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)
 	}

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

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.