Skip to content

Commit 6f63a8c

Browse files
warku123warku123
authored
fix(db): resolve resource leakage in CheckPointV2Store close() (tronprotocol#6688)
1 parent 636667a commit 6f63a8c

6 files changed

Lines changed: 238 additions & 10 deletions

File tree

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,25 @@ public void reset() {
7676
@Override
7777
public void close() {
7878
logger.info("******** Begin to close {}. ********", getName());
79+
doClose();
80+
logger.info("******** End to close {}. ********", getName());
81+
}
82+
83+
/**
84+
* Releases writeOptions and dbSource (best-effort, exceptions logged at WARN).
85+
* Subclasses with extra resources should override {@link #close()} and call
86+
* {@code doClose()} directly — not {@code super.close()} — to avoid duplicated logs.
87+
*/
88+
protected void doClose() {
7989
try {
8090
writeOptions.close();
91+
} catch (Exception e) {
92+
logger.warn("Failed to close writeOptions in {}.", getName(), e);
93+
}
94+
try {
8195
dbSource.closeDB();
8296
} catch (Exception e) {
83-
logger.warn("Failed to close {}.", getName(), e);
84-
} finally {
85-
logger.info("******** End to close {}. ********", getName());
97+
logger.warn("Failed to close dbSource in {}.", getName(), e);
8698
}
8799
}
88100

‎chainbase/src/main/java/org/tron/core/store/CheckPointV2Store.java‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,20 +62,16 @@ public void updateByBatch(Map<byte[], byte[]> rows) {
6262
this.dbSource.updateByBatch(rows, writeOptions);
6363
}
6464

65-
/**
66-
* close the database.
67-
*/
6865
@Override
6966
public void close() {
7067
logger.debug("******** Begin to close {}. ********", getName());
7168
try {
7269
writeOptions.close();
73-
dbSource.closeDB();
7470
} catch (Exception e) {
75-
logger.warn("Failed to close {}.", getName(), e);
76-
} finally {
77-
logger.debug("******** End to close {}. ********", getName());
71+
logger.warn("Failed to close writeOptions in {}.", getName(), e);
7872
}
73+
doClose();
74+
logger.debug("******** End to close {}. ********", getName());
7975
}
8076

8177
}

‎framework/src/test/java/org/tron/common/logsfilter/FilterQueryTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.List;
1818
import java.util.Map;
1919
import lombok.extern.slf4j.Slf4j;
20+
import org.junit.After;
2021
import org.junit.Assert;
2122
import org.junit.Test;
2223
import org.tron.common.logsfilter.capsule.ContractEventTriggerCapsule;
@@ -28,6 +29,11 @@
2829
@Slf4j
2930
public class FilterQueryTest {
3031

32+
@After
33+
public void tearDown() {
34+
EventPluginLoader.getInstance().setFilterQuery(null);
35+
}
36+
3137
@Test
3238
public synchronized void testParseFilterQueryBlockNumber() {
3339
assertEquals(LATEST_BLOCK_NUM, parseToBlockNumber(EMPTY));

‎framework/src/test/java/org/tron/core/db/CheckPointV2StoreTest.java‎

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
package org.tron.core.db;
2+
3+
import static org.mockito.Mockito.doThrow;
4+
import static org.mockito.Mockito.mock;
5+
import static org.mockito.Mockito.spy;
6+
import static org.mockito.Mockito.verify;
7+
8+
import java.io.IOException;
9+
import java.lang.reflect.Field;
10+
import org.junit.AfterClass;
11+
import org.junit.Assert;
12+
import org.junit.BeforeClass;
13+
import org.junit.ClassRule;
14+
import org.junit.Test;
15+
import org.junit.rules.TemporaryFolder;
16+
import org.rocksdb.RocksDB;
17+
import org.tron.common.TestConstants;
18+
import org.tron.common.storage.WriteOptionsWrapper;
19+
import org.tron.core.config.args.Args;
20+
import org.tron.core.db.common.DbSourceInter;
21+
import org.tron.core.store.CheckPointV2Store;
22+
23+
public class CheckPointV2StoreTest {
24+
25+
@ClassRule
26+
public static final TemporaryFolder temporaryFolder = new TemporaryFolder();
27+
28+
static {
29+
RocksDB.loadLibrary();
30+
}
31+
32+
@BeforeClass
33+
public static void initArgs() throws IOException {
34+
Args.setParam(
35+
new String[]{"-d", temporaryFolder.newFolder().toString()},
36+
TestConstants.TEST_CONF
37+
);
38+
}
39+
40+
@AfterClass
41+
public static void destroy() {
42+
Args.clearParam();
43+
}
44+
45+
@Test
46+
public void testStubMethods() throws Exception {
47+
CheckPointV2Store store = new CheckPointV2Store("test-stubs");
48+
try {
49+
byte[] key = "key".getBytes();
50+
51+
store.put(key, new byte[]{});
52+
Assert.assertNull(store.get(key));
53+
Assert.assertFalse(store.has(key));
54+
store.forEach(item -> {
55+
});
56+
Assert.assertNull(store.spliterator());
57+
58+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
59+
dbSourceField.setAccessible(true);
60+
DbSourceInter<byte[]> originalDbSource =
61+
(DbSourceInter<byte[]>) dbSourceField.get(store);
62+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
63+
dbSourceField.set(store, mockDbSource);
64+
store.delete(key);
65+
dbSourceField.set(store, originalDbSource);
66+
67+
java.lang.reflect.Method initMethod =
68+
CheckPointV2Store.class.getDeclaredMethod("init");
69+
initMethod.setAccessible(true);
70+
initMethod.invoke(store);
71+
} finally {
72+
store.close();
73+
}
74+
}
75+
76+
@Test
77+
public void testCloseWithRealResources() {
78+
CheckPointV2Store store = new CheckPointV2Store("test-close-real");
79+
// Exercises the real writeOptions.close() and dbSource.closeDB() code paths
80+
store.close();
81+
}
82+
83+
@Test
84+
public void testCloseReleasesAllResources() throws Exception {
85+
CheckPointV2Store store = new CheckPointV2Store("test-close");
86+
87+
// Replace dbSource with a mock so we can verify closeDB()
88+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
89+
dbSourceField.setAccessible(true);
90+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(store);
91+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
92+
dbSourceField.set(store, mockDbSource);
93+
94+
try {
95+
store.close();
96+
97+
verify(mockDbSource).closeDB();
98+
} finally {
99+
originalDbSource.closeDB();
100+
}
101+
}
102+
103+
@Test
104+
public void testCloseWhenDbSourceThrows() throws Exception {
105+
CheckPointV2Store store = new CheckPointV2Store("test-close-dbsource-throws");
106+
107+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
108+
dbSourceField.setAccessible(true);
109+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(store);
110+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
111+
doThrow(new RuntimeException("simulated dbSource failure")).when(mockDbSource).closeDB();
112+
dbSourceField.set(store, mockDbSource);
113+
114+
try {
115+
store.close();
116+
} finally {
117+
originalDbSource.closeDB();
118+
}
119+
}
120+
121+
@Test
122+
public void testCloseDbSourceWhenWriteOptionsThrows() throws Exception {
123+
CheckPointV2Store store = new CheckPointV2Store("test-close-exception");
124+
125+
// Replace child writeOptions with a spy that throws on close
126+
Field childWriteOptionsField = CheckPointV2Store.class.getDeclaredField("writeOptions");
127+
childWriteOptionsField.setAccessible(true);
128+
WriteOptionsWrapper childWriteOptions =
129+
(WriteOptionsWrapper) childWriteOptionsField.get(store);
130+
WriteOptionsWrapper spyChildWriteOptions = spy(childWriteOptions);
131+
doThrow(new RuntimeException("simulated writeOptions failure"))
132+
.when(spyChildWriteOptions).close();
133+
childWriteOptionsField.set(store, spyChildWriteOptions);
134+
135+
// Replace parent writeOptions with a spy that throws on close
136+
Field parentWriteOptionsField = TronDatabase.class.getDeclaredField("writeOptions");
137+
parentWriteOptionsField.setAccessible(true);
138+
WriteOptionsWrapper parentWriteOptions =
139+
(WriteOptionsWrapper) parentWriteOptionsField.get(store);
140+
WriteOptionsWrapper spyParentWriteOptions = spy(parentWriteOptions);
141+
doThrow(new RuntimeException("simulated parent writeOptions failure"))
142+
.when(spyParentWriteOptions).close();
143+
parentWriteOptionsField.set(store, spyParentWriteOptions);
144+
145+
// Replace dbSource with a mock
146+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
147+
dbSourceField.setAccessible(true);
148+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(store);
149+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
150+
dbSourceField.set(store, mockDbSource);
151+
152+
try {
153+
store.close();
154+
155+
// dbSource.closeDB() must be called even though both writeOptions threw
156+
verify(mockDbSource).closeDB();
157+
} finally {
158+
originalDbSource.closeDB();
159+
}
160+
}
161+
}

‎framework/src/test/java/org/tron/core/db/TronDatabaseTest.java‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
package org.tron.core.db;
22

3+
import static org.mockito.Mockito.doThrow;
4+
import static org.mockito.Mockito.mock;
5+
import static org.mockito.Mockito.spy;
6+
import static org.mockito.Mockito.verify;
7+
38
import com.google.protobuf.InvalidProtocolBufferException;
49
import java.io.IOException;
10+
import java.lang.reflect.Field;
511
import org.junit.AfterClass;
612
import org.junit.Assert;
713
import org.junit.BeforeClass;
@@ -12,7 +18,9 @@
1218
import org.junit.rules.TemporaryFolder;
1319
import org.rocksdb.RocksDB;
1420
import org.tron.common.TestConstants;
21+
import org.tron.common.storage.WriteOptionsWrapper;
1522
import org.tron.core.config.args.Args;
23+
import org.tron.core.db.common.DbSourceInter;
1624
import org.tron.core.exception.BadItemException;
1725
import org.tron.core.exception.ItemNotFoundException;
1826

@@ -99,4 +107,48 @@ public void TestGetFromRoot() throws
99107
Assert.assertEquals(db.getFromRoot("test".getBytes()),
100108
"test");
101109
}
110+
111+
@Test
112+
public void testDoCloseDbSourceCalledWhenWriteOptionsThrows() throws Exception {
113+
TronDatabase<String> db = new TronDatabase<String>("test-do-close") {
114+
115+
@Override
116+
public void put(byte[] key, String item) {
117+
}
118+
119+
@Override
120+
public void delete(byte[] key) {
121+
}
122+
123+
@Override
124+
public String get(byte[] key) {
125+
return null;
126+
}
127+
128+
@Override
129+
public boolean has(byte[] key) {
130+
return false;
131+
}
132+
};
133+
134+
Field writeOptionsField = TronDatabase.class.getDeclaredField("writeOptions");
135+
writeOptionsField.setAccessible(true);
136+
WriteOptionsWrapper spyWriteOptions = spy((WriteOptionsWrapper) writeOptionsField.get(db));
137+
doThrow(new RuntimeException("simulated writeOptions failure")).when(spyWriteOptions).close();
138+
writeOptionsField.set(db, spyWriteOptions);
139+
140+
Field dbSourceField = TronDatabase.class.getDeclaredField("dbSource");
141+
dbSourceField.setAccessible(true);
142+
DbSourceInter<byte[]> originalDbSource = (DbSourceInter<byte[]>) dbSourceField.get(db);
143+
DbSourceInter<byte[]> mockDbSource = mock(DbSourceInter.class);
144+
dbSourceField.set(db, mockDbSource);
145+
146+
try {
147+
db.doClose();
148+
verify(spyWriteOptions).close();
149+
verify(mockDbSource).closeDB();
150+
} finally {
151+
originalDbSource.closeDB();
152+
}
153+
}
102154
}

‎framework/src/test/java/org/tron/core/event/BlockEventGetTest.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public static void init() {
9090

9191
@Before
9292
public void before() throws IOException {
93+
EventPluginLoader.getInstance().setFilterQuery(null);
9394
Args.getInstance().setNodeListenPort(10000 + port.incrementAndGet());
9495

9596
dbManager = context.getBean(Manager.class);

0 commit comments

Comments
 (0)