From 870dc706ca73f49d7d6e2bfbfdd8bc9148055eed Mon Sep 17 00:00:00 2001
From: Sanjay Malakar <iamsanjaymalakar@gmail.com>
Date: Fri, 28 Aug 2026 01:22:11 -0700
Subject: [PATCH] HBASE-30345: Ensure FileIOEngine.shutdown() closes every
 backing file

FileIOEngine.shutdown() closed the FileChannel and the RandomAccessFile for
each backing file inside a single try block. If FileChannel.close() threw
IOException, the matching RandomAccessFile.close() was skipped and that file
handle was leaked. shutdown() also runs from the FileIOEngine constructor's
failure path, so a partially initialized engine leaked the same way.

Close each resource independently through a closeAll() helper. shutdown()
keeps logging the IOException at ERROR rather than throwing it.
---
 .../hbase/io/hfile/bucket/FileIOEngine.java   | 25 +++++++++++++------
 .../io/hfile/bucket/TestFileIOEngine.java     | 22 ++++++++++++++++
 2 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java
index ba431c4c6dcb..e7e6fd9a7278 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java
@@ -17,6 +17,7 @@
  */
 package org.apache.hadoop.hbase.io.hfile.bucket;
 
+import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
 import java.io.RandomAccessFile;
@@ -225,15 +226,23 @@ public void sync() throws IOException {
   @Override
   public void shutdown() {
     for (int i = 0; i < filePaths.length; i++) {
+      closeAll(filePaths[i], fileChannels[i], rafs[i]);
+    }
+  }
+
+  /**
+   * Close each non-null closeable, logging failures without throwing, so one failure cannot skip
+   * the rest.
+   */
+  private static void closeAll(String filePath, Closeable... closeables) {
+    for (Closeable c : closeables) {
+      if (c == null) {
+        continue;
+      }
       try {
-        if (fileChannels[i] != null) {
-          fileChannels[i].close();
-        }
-        if (rafs[i] != null) {
-          rafs[i].close();
-        }
-      } catch (IOException ex) {
-        LOG.error("Failed closing " + filePaths[i] + " when shudown the IOEngine", ex);
+        c.close();
+      } catch (IOException e) {
+        LOG.error("Failed closing {} when shutting down the IOEngine", filePath, e);
       }
     }
   }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java
index f19d13d8490e..66d3d80cd58d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java
@@ -27,6 +27,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.io.RandomAccessFile;
+import java.lang.reflect.Field;
 import java.nio.ByteBuffer;
 import java.nio.channels.FileChannel;
 import java.util.ArrayList;
@@ -192,4 +194,24 @@ public void testRefreshFileConnection() throws IOException {
       assertEquals(fileChannels[i], reopenedFileChannels[i]);
     }
   }
+
+  @Test
+  public void testShutdownClosesRandomAccessFileWhenChannelCloseFails() throws Exception {
+    FileChannel[] channels = fileIOEngine.getFileChannels();
+
+    // rafs is private
+    Field rafsField = FileIOEngine.class.getDeclaredField("rafs");
+    rafsField.setAccessible(true);
+    RandomAccessFile[] rafs = (RandomAccessFile[]) rafsField.get(fileIOEngine);
+
+    FileChannel failingChannel = Mockito.mock(FileChannel.class);
+    Mockito.doThrow(new IOException("channel close failed")).when(failingChannel).close();
+    RandomAccessFile raf = Mockito.mock(RandomAccessFile.class);
+    channels[0] = failingChannel;
+    rafs[0] = raf;
+
+    fileIOEngine.shutdown();
+
+    Mockito.verify(raf).close();
+  }
 }