Skip to content

Commit 7142e2e

Browse files
committed
fix(db): split try-catch in TronDatabase.close() and improve tests
Split the single try-catch block in TronDatabase.close() into separate blocks for writeOptions and dbSource to ensure dbSource.closeDB() runs even when writeOptions.close() throws an exception. Rewrite tests to verify the exception scenario: mock writeOptions to throw and assert that dbSource.closeDB() is still called.
1 parent 57949ca commit 7142e2e

2 files changed

Lines changed: 59 additions & 58 deletions

File tree

chainbase/src/main/java/org/tron/core/db/TronDatabase.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,15 @@ public void close() {
7878
logger.info("******** Begin to close {}. ********", getName());
7979
try {
8080
writeOptions.close();
81+
} catch (Exception e) {
82+
logger.warn("Failed to close writeOptions in {}.", getName(), e);
83+
}
84+
try {
8185
dbSource.closeDB();
8286
} catch (Exception e) {
83-
logger.warn("Failed to close {}.", getName(), e);
84-
} finally {
85-
logger.info("******** End to close {}. ********", getName());
87+
logger.warn("Failed to close dbSource in {}.", getName(), e);
8688
}
89+
logger.info("******** End to close {}. ********", getName());
8790
}
8891

8992
public abstract void put(byte[] key, T item);
Lines changed: 53 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
package org.tron.core.db;
22

3+
import static org.mockito.Mockito.doThrow;
4+
import static org.mockito.Mockito.mock;
35
import static org.mockito.Mockito.spy;
4-
import static org.mockito.Mockito.times;
56
import static org.mockito.Mockito.verify;
67

78
import java.io.IOException;
89
import java.lang.reflect.Field;
910
import org.junit.AfterClass;
10-
import org.junit.Assert;
1111
import org.junit.BeforeClass;
1212
import org.junit.ClassRule;
1313
import org.junit.Test;
@@ -16,11 +16,9 @@
1616
import org.tron.common.TestConstants;
1717
import org.tron.common.storage.WriteOptionsWrapper;
1818
import org.tron.core.config.args.Args;
19+
import org.tron.core.db.common.DbSourceInter;
1920
import org.tron.core.store.CheckPointV2Store;
2021

21-
/**
22-
* Unit tests for {@link CheckPointV2Store}.
23-
*/
2422
public class CheckPointV2StoreTest {
2523

2624
@ClassRule
@@ -30,11 +28,6 @@ public class CheckPointV2StoreTest {
3028
RocksDB.loadLibrary();
3129
}
3230

33-
/**
34-
* Initializes test arguments before running all tests.
35-
*
36-
* @throws IOException if an I/O error occurs
37-
*/
3831
@BeforeClass
3932
public static void initArgs() throws IOException {
4033
Args.setParam(
@@ -43,64 +36,69 @@ public static void initArgs() throws IOException {
4336
);
4437
}
4538

46-
/**
47-
* Clears test arguments after all tests have run.
48-
*/
4939
@AfterClass
5040
public static void destroy() {
5141
Args.clearParam();
5242
}
5343

54-
/**
55-
* Tests that {@link CheckPointV2Store#close()} properly calls {@code super.close()}
56-
* to ensure parent resources are released.
57-
*
58-
* @throws Exception if an error occurs during the test
59-
*/
6044
@Test
61-
public void testCloseCallsSuperClose() throws Exception {
62-
CheckPointV2Store store = new CheckPointV2Store("test-close-super");
63-
64-
// Get the parent class's writeOptions field
45+
public void testCloseReleasesAllResources() throws Exception {
46+
CheckPointV2Store store = new CheckPointV2Store("test-close");
47+
48+
// Replace dbSource with a mock so we can verify closeDB()
49+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
50+
dbSourceField.setAccessible(true);
51+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(store);
52+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
53+
dbSourceField.set(store, mockDbSource);
54+
55+
try {
56+
store.close();
57+
58+
verify(mockDbSource).closeDB();
59+
} finally {
60+
originalDbSource.closeDB();
61+
}
62+
}
63+
64+
@Test
65+
public void testCloseDbSourceWhenWriteOptionsThrows() throws Exception {
66+
CheckPointV2Store store = new CheckPointV2Store("test-close-exception");
67+
68+
// Replace child writeOptions with a spy that throws on close
69+
Field childWriteOptionsField = CheckPointV2Store.class.getDeclaredField("writeOptions");
70+
childWriteOptionsField.setAccessible(true);
71+
WriteOptionsWrapper childWriteOptions =
72+
(WriteOptionsWrapper) childWriteOptionsField.get(store);
73+
WriteOptionsWrapper spyChildWriteOptions = spy(childWriteOptions);
74+
doThrow(new RuntimeException("simulated writeOptions failure"))
75+
.when(spyChildWriteOptions).close();
76+
childWriteOptionsField.set(store, spyChildWriteOptions);
77+
78+
// Replace parent writeOptions with a spy that throws on close
6579
Field parentWriteOptionsField = TronDatabase.class.getDeclaredField("writeOptions");
6680
parentWriteOptionsField.setAccessible(true);
67-
WriteOptionsWrapper originalParentWriteOptions =
81+
WriteOptionsWrapper parentWriteOptions =
6882
(WriteOptionsWrapper) parentWriteOptionsField.get(store);
69-
70-
// Save the original rocks object reference for subsequent verification
71-
org.rocksdb.WriteOptions originalRocks = originalParentWriteOptions.rocks;
72-
73-
// Create a spy to monitor the parent class's writeOptions
74-
WriteOptionsWrapper spyParentWriteOptions = spy(originalParentWriteOptions);
83+
WriteOptionsWrapper spyParentWriteOptions = spy(parentWriteOptions);
84+
doThrow(new RuntimeException("simulated parent writeOptions failure"))
85+
.when(spyParentWriteOptions).close();
7586
parentWriteOptionsField.set(store, spyParentWriteOptions);
76-
77-
// Create a spy to monitor the rocks.close() method
78-
org.rocksdb.WriteOptions spyRocks = spy(originalRocks);
79-
spyParentWriteOptions.rocks = spyRocks;
80-
87+
88+
// Replace dbSource with a mock
89+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
90+
dbSourceField.setAccessible(true);
91+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(store);
92+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
93+
dbSourceField.set(store, mockDbSource);
94+
8195
try {
82-
// Verify that the parent class's writeOptions and dbSource exist
83-
Assert.assertNotNull(spyParentWriteOptions);
84-
Assert.assertNotNull(spyRocks);
85-
Assert.assertNotNull(store.getDbSource());
86-
87-
// Close the store
8896
store.close();
89-
90-
// Verify that the parent class's writeOptions.close() was called (via super.close())
91-
verify(spyParentWriteOptions, times(1)).close();
92-
93-
// Verify that rocks.close() was called (resources are actually closed)
94-
verify(spyRocks, times(1)).close();
97+
98+
// dbSource.closeDB() must be called even though both writeOptions threw
99+
verify(mockDbSource).closeDB();
95100
} finally {
96-
// Cleanup: restore original state to avoid cross-test contamination
97-
if (store != null) {
98-
store.close();
99-
}
100-
// Restore original writeOptions to remove spies
101-
parentWriteOptionsField.set(store, originalParentWriteOptions);
102-
// Restore original rocks reference
103-
originalParentWriteOptions.rocks = originalRocks;
101+
originalDbSource.closeDB();
104102
}
105103
}
106104
}

0 commit comments

Comments
 (0)