From 2bff24274b6f480a85aaa04b10286fb020d03123 Mon Sep 17 00:00:00 2001 From: warku123 Date: Thu, 10 Sep 2026 17:39:48 +0800 Subject: [PATCH 1/3] test: add remaining test isolation fixes --- .../java/org/tron/common/BaseMethodTest.java | 2 + .../test/java/org/tron/common/BaseTest.java | 7 ++ .../common/runtime/vm/AllowTvmLondonTest.java | 4 +- .../utils/PeerManagerStateResetter.java | 68 +++++++++++++++++++ .../messagehandler/MessageHandlerTest.java | 2 + .../messagehandler/PbftMsgHandlerTest.java | 2 + .../tron/core/net/peer/PeerManagerTest.java | 2 + .../net/services/HandShakeServiceTest.java | 2 + .../org/tron/core/services/WalletApiTest.java | 2 + .../services/stop/ConditionallyStopTest.java | 2 +- .../tron/core/zksnark/SendCoinShieldTest.java | 10 +++ 11 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java diff --git a/framework/src/test/java/org/tron/common/BaseMethodTest.java b/framework/src/test/java/org/tron/common/BaseMethodTest.java index 6fc02d3badc..4c5d6d53b12 100644 --- a/framework/src/test/java/org/tron/common/BaseMethodTest.java +++ b/framework/src/test/java/org/tron/common/BaseMethodTest.java @@ -10,6 +10,7 @@ import org.tron.common.application.Application; import org.tron.common.application.ApplicationFactory; import org.tron.common.application.TronApplicationContext; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.core.ChainBaseManager; import org.tron.core.config.DefaultConfig; import org.tron.core.config.args.Args; @@ -60,6 +61,7 @@ protected String configFile() { @Before public final void initContext() throws IOException { + PeerManagerStateResetter.reset(); String[] baseArgs = new String[]{ "--output-directory", temporaryFolder.newFolder().toString()}; String[] allArgs = mergeArgs(baseArgs, extraArgs()); diff --git a/framework/src/test/java/org/tron/common/BaseTest.java b/framework/src/test/java/org/tron/common/BaseTest.java index 23967be542e..743bca71044 100644 --- a/framework/src/test/java/org/tron/common/BaseTest.java +++ b/framework/src/test/java/org/tron/common/BaseTest.java @@ -7,6 +7,7 @@ import lombok.extern.slf4j.Slf4j; import org.junit.AfterClass; import org.junit.Assert; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.rules.TemporaryFolder; @@ -18,6 +19,7 @@ import org.tron.common.crypto.ECKey; import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.Commons; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.Sha256Hash; import org.tron.consensus.base.Param; import org.tron.core.ChainBaseManager; @@ -79,6 +81,11 @@ public abstract class BaseTest { private static Application appT1; + @Before + public void resetPeerManagerState() { + PeerManagerStateResetter.reset(); + } + @PostConstruct private void prepare() { appT1 = appT; diff --git a/framework/src/test/java/org/tron/common/runtime/vm/AllowTvmLondonTest.java b/framework/src/test/java/org/tron/common/runtime/vm/AllowTvmLondonTest.java index e93eca39092..11a02e615db 100644 --- a/framework/src/test/java/org/tron/common/runtime/vm/AllowTvmLondonTest.java +++ b/framework/src/test/java/org/tron/common/runtime/vm/AllowTvmLondonTest.java @@ -74,8 +74,8 @@ public void testBaseFee() throws ContractExeException, ReceiptCheckErrException, factoryAddress, Hex.decode(hexInput), 0, feeLimit, manager, null); byte[] returnValue = result.getRuntime().getResult().getHReturn(); Assert.assertNull(result.getRuntime().getRuntimeError()); - Assert.assertArrayEquals(returnValue, - longTo32Bytes(manager.getDynamicPropertiesStore().getEnergyFee())); + Assert.assertArrayEquals(longTo32Bytes(manager.getDynamicPropertiesStore().getEnergyFee()), + returnValue); } @Test diff --git a/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java b/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java new file mode 100644 index 00000000000..471e4f53ee0 --- /dev/null +++ b/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java @@ -0,0 +1,68 @@ +package org.tron.common.utils; + +import java.lang.reflect.Field; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.atomic.AtomicInteger; +import org.springframework.util.ReflectionUtils; +import org.tron.common.es.ExecutorServiceManager; +import org.tron.core.net.peer.PeerConnection; +import org.tron.core.net.peer.PeerManager; + +/** + * Test source-set utility for restoring PeerManager to a cold-start state. + * + *

{@link PeerManager#close()} disconnects visible peers but does not clear its raw static + * peers or counters. Its static executor also remains shut down and is not rebuilt by normal + * initialization. Tests reuse one JVM across Spring contexts, so this reset is needed before + * another test starts. + * + *

The current BaseTest/BaseMethodTest wiring is intentionally broad to protect tests that + * reuse Spring contexts or the JVM from unknown preceding pollution. For tests that do not use + * PeerManager, reset is idempotent and low-impact: it typically clears an empty list and zeros + * counters, while the executor is replaced only when dead or null and threads are created on + * demand. If production PeerManager lifecycle becomes restart-safe, this wiring can be narrowed + * or this utility can be removed in the future. + */ +public final class PeerManagerStateResetter { + + private static final String EXECUTOR_NAME = "peer-manager"; + + private PeerManagerStateResetter() { + } + + public static synchronized void reset() { + List peers = getFieldValue("peers"); + if (peers != null) { + peers.clear(); + } else { + setFieldValue("peers", + Collections.synchronizedList(new java.util.ArrayList())); + } + + AtomicInteger active = PeerManager.getActivePeersCount(); + AtomicInteger passive = PeerManager.getPassivePeersCount(); + active.set(0); + passive.set(0); + + ScheduledExecutorService executor = + getFieldValue("executor"); + if (executor == null || executor.isShutdown() || executor.isTerminated()) { + setFieldValue("executor", + ExecutorServiceManager.newSingleThreadScheduledExecutor(EXECUTOR_NAME)); + } + } + + private static T getFieldValue(String fieldName) { + Field field = ReflectionUtils.findField(PeerManager.class, fieldName); + ReflectionUtils.makeAccessible(field); + return (T) ReflectionUtils.getField(field, null); + } + + private static void setFieldValue(String fieldName, Object value) { + Field field = ReflectionUtils.findField(PeerManager.class, fieldName); + ReflectionUtils.makeAccessible(field); + ReflectionUtils.setField(field, null, value); + } +} diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/MessageHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/MessageHandlerTest.java index be843674632..c8205b6b721 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/MessageHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/MessageHandlerTest.java @@ -16,6 +16,7 @@ import org.tron.common.ClassLevelAppContextFixture; import org.tron.common.TestConstants; import org.tron.common.application.TronApplicationContext; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; import org.tron.consensus.pbft.message.PbftMessage; @@ -45,6 +46,7 @@ public class MessageHandlerTest { @BeforeClass public static void init() throws Exception { + PeerManagerStateResetter.reset(); Args.setParam(new String[] {"--output-directory", temporaryFolder.newFolder().toString(), "--debug"}, TestConstants.TEST_CONF); context = APP_FIXTURE.createContext(); diff --git a/framework/src/test/java/org/tron/core/net/messagehandler/PbftMsgHandlerTest.java b/framework/src/test/java/org/tron/core/net/messagehandler/PbftMsgHandlerTest.java index 65a8f615bfe..15d7107b58f 100644 --- a/framework/src/test/java/org/tron/core/net/messagehandler/PbftMsgHandlerTest.java +++ b/framework/src/test/java/org/tron/core/net/messagehandler/PbftMsgHandlerTest.java @@ -17,6 +17,7 @@ import org.tron.common.crypto.SignInterface; import org.tron.common.crypto.SignUtils; import org.tron.common.utils.FileUtil; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.PublicMethod; import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; @@ -46,6 +47,7 @@ public class PbftMsgHandlerTest { @BeforeClass public static void init() { + PeerManagerStateResetter.reset(); Args.setParam(new String[] {"--output-directory", dbPath, "--debug"}, TestConstants.TEST_CONF); context = new TronApplicationContext(DefaultConfig.class); diff --git a/framework/src/test/java/org/tron/core/net/peer/PeerManagerTest.java b/framework/src/test/java/org/tron/core/net/peer/PeerManagerTest.java index ffba127a6fd..16e88b38584 100644 --- a/framework/src/test/java/org/tron/core/net/peer/PeerManagerTest.java +++ b/framework/src/test/java/org/tron/core/net/peer/PeerManagerTest.java @@ -17,6 +17,7 @@ import org.springframework.context.ApplicationContext; import org.tron.common.TestConstants; import org.tron.common.parameter.CommonParameter; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.ReflectUtils; import org.tron.core.config.args.Args; import org.tron.p2p.connection.Channel; @@ -25,6 +26,7 @@ public class PeerManagerTest { @BeforeClass public static void initArgs() { + PeerManagerStateResetter.reset(); Args.setParam(new String[]{}, TestConstants.TEST_CONF); CommonParameter.getInstance().setRateLimiterSyncBlockChain(10); CommonParameter.getInstance().setRateLimiterFetchInvData(10); diff --git a/framework/src/test/java/org/tron/core/net/services/HandShakeServiceTest.java b/framework/src/test/java/org/tron/core/net/services/HandShakeServiceTest.java index 3a0fc74c73f..30792f73bae 100644 --- a/framework/src/test/java/org/tron/core/net/services/HandShakeServiceTest.java +++ b/framework/src/test/java/org/tron/core/net/services/HandShakeServiceTest.java @@ -19,6 +19,7 @@ import org.springframework.context.ApplicationContext; import org.tron.common.TestConstants; import org.tron.common.application.TronApplicationContext; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.ReflectUtils; import org.tron.common.utils.Sha256Hash; import org.tron.core.ChainBaseManager; @@ -52,6 +53,7 @@ public class HandShakeServiceTest { @BeforeClass public static void init() throws Exception { + PeerManagerStateResetter.reset(); Args.setParam(new String[] {"--output-directory", temporaryFolder.newFolder().toString(), "--debug"}, TestConstants.TEST_CONF); context = new TronApplicationContext(DefaultConfig.class); diff --git a/framework/src/test/java/org/tron/core/services/WalletApiTest.java b/framework/src/test/java/org/tron/core/services/WalletApiTest.java index 4a55556afb1..25b21f30872 100644 --- a/framework/src/test/java/org/tron/core/services/WalletApiTest.java +++ b/framework/src/test/java/org/tron/core/services/WalletApiTest.java @@ -17,6 +17,7 @@ import org.tron.common.ClassLevelAppContextFixture; import org.tron.common.TestConstants; import org.tron.common.application.TronApplicationContext; +import org.tron.common.utils.PeerManagerStateResetter; import org.tron.common.utils.PublicMethod; import org.tron.common.utils.TimeoutInterceptor; import org.tron.core.config.args.Args; @@ -38,6 +39,7 @@ public class WalletApiTest { @BeforeClass public static void init() throws IOException { + PeerManagerStateResetter.reset(); Args.setParam(new String[] {"-d", temporaryFolder.newFolder().toString(), "--p2p-disable", "true"}, TestConstants.TEST_CONF); Args.getInstance().setRpcPort(PublicMethod.chooseRandomPort()); diff --git a/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java b/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java index 964cfbf254d..0f398c64c0a 100644 --- a/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java +++ b/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java @@ -105,7 +105,7 @@ private void generateBlock() throws Exception { chainManager.getDynamicPropertiesStore().getLatestBlockHeaderNumberFromDB()); } - @Test(timeout = 30_000) // milliseconds + @Test(timeout = 45_000) // milliseconds public void testStop() throws Exception { while (!tronNetDelegate.isHitDown()) { generateBlock(); diff --git a/framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java b/framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java index 08de83ca8bf..efa60139b12 100644 --- a/framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java +++ b/framework/src/test/java/org/tron/core/zksnark/SendCoinShieldTest.java @@ -14,6 +14,7 @@ import java.util.Optional; import javax.annotation.Resource; import lombok.extern.slf4j.Slf4j; +import org.junit.After; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; @@ -106,6 +107,7 @@ public class SendCoinShieldTest extends BaseTest { private static final int VOTE_SCORE = 2; private static final String DESCRIPTION = "TRX"; private static final String URL = "https://tron.network"; + private long previousAllowShieldedTransaction; @Resource private Wallet wallet; @@ -130,6 +132,8 @@ public static void initZksnarkParams() { */ @Before public void init() { + previousAllowShieldedTransaction = dbManager.getDynamicPropertiesStore() + .getAllowShieldedTransaction(); if (init) { return; } @@ -155,6 +159,12 @@ public void init() { init = true; } + @After + public void restoreAllowShieldedTransaction() { + dbManager.getDynamicPropertiesStore() + .saveAllowShieldedTransaction(previousAllowShieldedTransaction); + } + private void addZeroValueOutputNote(ZenTransactionBuilder builder) throws ZksnarkException { SpendingKey spendingKey = SpendingKey.random(); FullViewingKey fullViewingKey = spendingKey.fullViewingKey(); From aaed0df7f6faf19c35ab045b031c63a4a9af3933 Mon Sep 17 00:00:00 2001 From: warku123 Date: Thu, 10 Sep 2026 18:27:55 +0800 Subject: [PATCH 2/3] test: harden PeerManager state reset against task races --- .../utils/PeerManagerStateResetter.java | 67 +++++++++++++++---- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java b/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java index 471e4f53ee0..0103691938a 100644 --- a/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java +++ b/framework/src/test/java/org/tron/common/utils/PeerManagerStateResetter.java @@ -1,6 +1,7 @@ package org.tron.common.utils; import java.lang.reflect.Field; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.concurrent.ScheduledExecutorService; @@ -9,6 +10,7 @@ import org.tron.common.es.ExecutorServiceManager; import org.tron.core.net.peer.PeerConnection; import org.tron.core.net.peer.PeerManager; +import org.tron.protos.Protocol.ReasonCode; /** * Test source-set utility for restoring PeerManager to a cold-start state. @@ -18,11 +20,33 @@ * initialization. Tests reuse one JVM across Spring contexts, so this reset is needed before * another test starts. * + *

The executor is drained first because {@code check()} is not synchronized: it iterates a + * snapshot of the peers list and then removes entries and decrements the counters. A reset + * that only cleared the list and counters while an old {@code check()} task was still running + * or queued would let that task decrement the freshly zeroed counters afterwards, leaving + * negative counts. {@code synchronized} on reset alone cannot prevent this, so the old + * executor must be shut down and awaited before any state is touched. + * + *

The peers list is only cleared after a best-effort disconnect of its live entries. + * There is no dedicated teardown path, and {@link PeerManager#close()} may fail midway, so + * the raw list can still hold peers whose channels are alive; a bare {@code clear()} would + * orphan those connections. Each peer is disconnected individually and defensively (null + * channel tolerated, per-peer try/catch) so that one broken entry cannot abort the cleanup. + * + *

For tests that exercise p2p, reset now terminates any already scheduled + * {@code check()}/{@code logPeerStats()} tasks and installs a fresh executor (its thread is + * created lazily, and no task is scheduled again until the next {@code init()}). This is safe + * for tests: {@code check()} only removes peers whose channel has been half-disconnected for + * over 60 seconds and {@code logPeerStats()} only logs metrics, and no test depends on + * either. Reset also does not interlock with a concurrently running {@code init()} or + * {@code close()}; the sequential test lifecycle keeps that window out of reach. + * *

The current BaseTest/BaseMethodTest wiring is intentionally broad to protect tests that * reuse Spring contexts or the JVM from unknown preceding pollution. For tests that do not use * PeerManager, reset is idempotent and low-impact: it typically clears an empty list and zeros - * counters, while the executor is replaced only when dead or null and threads are created on - * demand. If production PeerManager lifecycle becomes restart-safe, this wiring can be narrowed + * counters, while the executor swap is cheap (an idle old executor stops immediately and the + * fresh executor only creates its thread on demand). If production PeerManager lifecycle + * becomes restart-safe, this wiring can be narrowed * or this utility can be removed in the future. */ public final class PeerManagerStateResetter { @@ -33,25 +57,42 @@ private PeerManagerStateResetter() { } public static synchronized void reset() { + // 1) Drain the old executor first: let running/queued check() tasks die out so they + // cannot interleave with the list/counter reset below. + ScheduledExecutorService executor = getFieldValue("executor"); + if (executor != null && !executor.isShutdown()) { + ExecutorServiceManager.shutdownAndAwaitTermination(executor, EXECUTOR_NAME); + } + // 2) Unconditionally install a fresh executor (the old one may be shut down or null); + // its thread is created lazily. + setFieldValue("executor", + ExecutorServiceManager.newSingleThreadScheduledExecutor(EXECUTOR_NAME)); + + // 3) Release residual live connections before clearing the raw list. List peers = getFieldValue("peers"); - if (peers != null) { - peers.clear(); + if (peers == null) { + setFieldValue("peers", Collections.synchronizedList(new ArrayList())); } else { - setFieldValue("peers", - Collections.synchronizedList(new java.util.ArrayList())); + for (PeerConnection peer : new ArrayList<>(peers)) { + try { + if (!peer.isDisconnect()) { + peer.disconnect(ReasonCode.PEER_QUITING); + if (peer.getChannel() != null) { + peer.getChannel().close(); + } + } + } catch (Exception e) { + // best effort: a single corrupted leftover peer must not fail the reset + } + } + peers.clear(); } + // 4) Zero the counters; old tasks can no longer decrement them at this point. AtomicInteger active = PeerManager.getActivePeersCount(); AtomicInteger passive = PeerManager.getPassivePeersCount(); active.set(0); passive.set(0); - - ScheduledExecutorService executor = - getFieldValue("executor"); - if (executor == null || executor.isShutdown() || executor.isTerminated()) { - setFieldValue("executor", - ExecutorServiceManager.newSingleThreadScheduledExecutor(EXECUTOR_NAME)); - } } private static T getFieldValue(String fieldName) { From 8b23ebb4af40d026182390a455624cb463024026 Mon Sep 17 00:00:00 2001 From: warku123 Date: Fri, 11 Sep 2026 11:16:18 +0800 Subject: [PATCH 3/3] test: restore stop-test timeout to 30 seconds --- .../java/org/tron/core/services/stop/ConditionallyStopTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java b/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java index 0f398c64c0a..964cfbf254d 100644 --- a/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java +++ b/framework/src/test/java/org/tron/core/services/stop/ConditionallyStopTest.java @@ -105,7 +105,7 @@ private void generateBlock() throws Exception { chainManager.getDynamicPropertiesStore().getLatestBlockHeaderNumberFromDB()); } - @Test(timeout = 45_000) // milliseconds + @Test(timeout = 30_000) // milliseconds public void testStop() throws Exception { while (!tronNetDelegate.isHitDown()) { generateBlock();