HDDS-16654. Replace usage of deprecated finalize() in OM

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshot.java

@@ -29,6 +29,7 @@
 import java.util.stream.Collectors;
 import org.apache.hadoop.hdds.client.RatisReplicationConfig;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.LeakDetector;
 import org.apache.hadoop.ozone.OzoneAcl;
 import org.apache.hadoop.ozone.audit.AuditLogger;
 import org.apache.hadoop.ozone.audit.AuditLoggerType;
@@ -48,6 +49,7 @@
 import org.apache.hadoop.ozone.security.acl.OzoneObj;
 import org.apache.hadoop.ozone.security.acl.OzoneObjInfo;
 import org.apache.hadoop.util.Time;
+import org.apache.ratis.util.UncheckedAutoCloseable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -72,6 +74,8 @@ public class OmSnapshot implements IOmMetadataReader, Closeable {
   private static final AuditLogger AUDIT = new AuditLogger(
       AuditLoggerType.OMLOGGER);
 
+  private static final LeakDetector LEAK_DETECTOR = new LeakDetector(OmSnapshot.class.getName());
+
   private final OmMetadataReader omMetadataReader;
   private final String volumeName;
   private final String bucketName;
@@ -80,6 +84,8 @@ public class OmSnapshot implements IOmMetadataReader, Closeable {
   // To access snapshot checkpoint DB metadata
   private final OMMetadataManager omMetadataManager;
   private final KeyManager keyManager;
+  /** Warns if this snapshot is garbage collected without being closed. Assigned in the constructor. */
+  private final UncheckedAutoCloseable leakTracker;
 
   public OmSnapshot(KeyManager keyManager,
                     PrefixManager prefixManager,
@@ -100,6 +106,8 @@ public OmSnapshot(KeyManager keyManager,
     this.snapshotID = snapshotID;
     this.keyManager = keyManager;
     this.omMetadataManager = keyManager.getMetadataManager();
+    this.leakTracker = LEAK_DETECTOR.track(this,
+        newLeakReporter(omMetadataManager.getStore().toString(), snapshotName));
   }
 
   @Override
@@ -343,21 +351,21 @@ public UUID getSnapshotID() {
 
   @Override
   public void close() throws IOException {
-    // Close DB
-    omMetadataManager.getStore().close();
+    try {
+      // Close DB
+      omMetadataManager.getStore().close();
+    } finally {
+      // Closed properly: stop tracking so the leak reporter does not fire at GC.
+      leakTracker.close();
+    }
   }
 
-  @Override
-  protected void finalize() throws Throwable {
-    // Verify that the DB handle has been closed, log warning otherwise
-    // https://softwareengineering.stackexchange.com/a/288724
-    if (!omMetadataManager.getStore().isClosed()) {
-      LOG.warn("{} is not closed properly. snapshotName: {}",
-          // Print hash code for debugging
-          omMetadataManager.getStore().toString(),
-          snapshotName);
-    }
-    super.finalize();
+  /**
+   * @return a leak reporter that captures only the store's hashcode and the snapshot name,
+   *     never the {@link OmSnapshot} itself.
+   */
+  static Runnable newLeakReporter(String storeHashCode, String snapshotName) {
+    return () -> LOG.warn("{} is not closed properly. snapshotName: {}", storeHashCode, snapshotName);
   }
 
   @VisibleForTesting

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLeakDetection.java

@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.UUID;
+import org.apache.hadoop.hdds.utils.db.DBStore;
+import org.apache.hadoop.ozone.security.acl.IAccessAuthorizer;
+import org.apache.ozone.test.GenericTestUtils.LogCapturer;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test {@link OmSnapshot}'s leak detection: the snapshot registers with a shared LeakDetector
+ * and warns if it is garbage collected while its underlying store is still open.
+ */
+class TestOmSnapshotLeakDetection {
+
+  /**
+   * Drive an actual {@link OmSnapshot} instance through GC without closing it and verify the leak
+   * is detected. Collaborators (including the {@link DBStore}) are mocked, so no metadata store is
+   * opened; this exercises the constructor's LeakDetector registration and the GC-triggered report.
+   */
+  @Test
+  void leakDetectedForUnclosedSnapshot() throws Exception {
+    try (LogCapturer logs = LogCapturer.captureLogs(OmSnapshot.class)) {
+      OmSnapshot snapshot = newSnapshotWithMockedStore();
+      assertThat(snapshot).isNotNull();
+
+      // Drop the only strong reference; the reporter captures no reference back to the snapshot,
+      // so it becomes collectible. The report runs asynchronously on the LeakDetector thread.
+      snapshot = null;
+      for (int i = 0; i < 50 && !logs.getOutput().contains("is not closed properly"); i++) {
+        System.gc();
+        Thread.sleep(100);
+      }
+      assertThat(logs.getOutput()).contains("is not closed properly. snapshotName: snap-1");
+    }
+  }
+
+  /** Closing the snapshot stops the leak tracker, so a GC afterwards must not report a leak. */
+  @Test
+  void closedSnapshotDoesNotReportLeak() throws Exception {
+    try (LogCapturer logs = LogCapturer.captureLogs(OmSnapshot.class)) {
+      OmSnapshot snapshot = newSnapshotWithMockedStore();
+      snapshot.close();
+
+      snapshot = null;
+      System.gc();
+      Thread.sleep(100);
+      assertThat(logs.getOutput()).doesNotContain("is not closed properly");
+    }
+  }
+
+  private static OmSnapshot newSnapshotWithMockedStore() {
+    DBStore store = mock(DBStore.class);
+    OMMetadataManager metadataManager = mock(OMMetadataManager.class);
+    when(metadataManager.getStore()).thenReturn(store);
+    KeyManager keyManager = mock(KeyManager.class);
+    when(keyManager.getMetadataManager()).thenReturn(metadataManager);
+    OzoneManager ozoneManager = mock(OzoneManager.class);
+    IAccessAuthorizer authorizer = mock(IAccessAuthorizer.class);
+    when(ozoneManager.getAccessAuthorizer()).thenReturn(authorizer);
+    when(authorizer.isNative()).thenReturn(false);
+    PrefixManager prefixManager = mock(PrefixManager.class);
+
+    return new OmSnapshot(keyManager, prefixManager, ozoneManager,
+        "vol", "bucket", "snap-1", UUID.randomUUID());
+  }
+}