From 9d73b815a8fd9ddab05869a59e3d21a67f2f27a6 Mon Sep 17 00:00:00 2001 From: wb Date: Wed, 3 Jun 2026 14:23:05 +0800 Subject: [PATCH] fix(merkle): build MerkleTree per-instance to fix concurrent race MerkleTree was a shared volatile singleton holding per-build mutable state (leaves/hashList/root). When merkle validation runs concurrently (e.g. pre-broadcast validation on the P2P handler threads alongside the apply path), concurrent calls mutated the shared leaves list and threw ArrayIndexOutOfBoundsException (surfaced on an ARM SR due to its weaker memory model), which escaped the BadBlockException catch and dropped peers. - MerkleTree: replace the getInstance() singleton with a static build() factory returning a thread-confined instance; drop the now-inaccurate @NotThreadSafe. - BlockCapsule.calcMerkleRoot: use MerkleTree.build(ids); keeps the config-driven hash engine, so no consensus behavior change. - MerkleTreeTest: switch call sites to build(); un-ignore testConcurrent and turn it into a regression test asserting 1000 concurrent builds succeed. --- .../org/tron/core/capsule/BlockCapsule.java | 2 +- .../tron/core/capsule/utils/MerkleTree.java | 29 +++++------------ .../core/capsule/utils/MerkleTreeTest.java | 31 ++++++++----------- 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java index 63acf64b64f..e6cbd52e595 100755 --- a/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java +++ b/chainbase/src/main/java/org/tron/core/capsule/BlockCapsule.java @@ -227,7 +227,7 @@ public Sha256Hash calcMerkleRoot() { .map(TransactionCapsule::getMerkleHash) .collect(Collectors.toCollection(ArrayList::new)); - return MerkleTree.getInstance().createTree(ids).getRoot().getHash(); + return MerkleTree.build(ids).getRoot().getHash(); } public void validateMerkleRoot() throws BadBlockException { diff --git a/chainbase/src/main/java/org/tron/core/capsule/utils/MerkleTree.java b/chainbase/src/main/java/org/tron/core/capsule/utils/MerkleTree.java index 94d22f4b474..cb6f299e872 100644 --- a/chainbase/src/main/java/org/tron/core/capsule/utils/MerkleTree.java +++ b/chainbase/src/main/java/org/tron/core/capsule/utils/MerkleTree.java @@ -5,41 +5,28 @@ import java.util.stream.Collectors; import java.util.stream.IntStream; import lombok.Getter; -import net.jcip.annotations.NotThreadSafe; import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.Sha256Hash; @Getter -@NotThreadSafe public class MerkleTree { - private static volatile MerkleTree instance; private List hashList; private List leaves; private Leaf root; - public static MerkleTree getInstance() { - if (instance == null) { - synchronized (MerkleTree.class) { - if (instance == null) { - instance = new MerkleTree(); - } - } - } - return instance; - } - - public MerkleTree createTree(List hashList) { - this.leaves = new ArrayList<>(); - this.hashList = hashList; - List leaves = createLeaves(hashList); + public static MerkleTree build(List hashList) { + MerkleTree tree = new MerkleTree(); + tree.hashList = hashList; + tree.leaves = new ArrayList<>(); + List leaves = tree.createLeaves(hashList); while (leaves.size() > 1) { - leaves = createParentLeaves(leaves); + leaves = tree.createParentLeaves(leaves); } - this.root = leaves.get(0); - return this; + tree.root = leaves.get(0); + return tree; } private List createParentLeaves(List leaves) { diff --git a/framework/src/test/java/org/tron/core/capsule/utils/MerkleTreeTest.java b/framework/src/test/java/org/tron/core/capsule/utils/MerkleTreeTest.java index 88e95f9653e..c9fea6bce45 100644 --- a/framework/src/test/java/org/tron/core/capsule/utils/MerkleTreeTest.java +++ b/framework/src/test/java/org/tron/core/capsule/utils/MerkleTreeTest.java @@ -10,10 +10,7 @@ import java.util.stream.IntStream; import lombok.extern.slf4j.Slf4j; import org.junit.Assert; -import org.junit.Ignore; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.tron.common.parameter.CommonParameter; import org.tron.common.utils.ByteArray; import org.tron.common.utils.MerkleRoot; @@ -23,9 +20,6 @@ @Slf4j public class MerkleTreeTest { - @Rule - public ExpectedException exception = ExpectedException.none(); - private static List getHash(int hashNum) { List hashList = new ArrayList(); for (int i = 0; i < hashNum; i++) { @@ -102,7 +96,7 @@ private static int getRank(int num) { public void test0HashNum() { List hashList = getHash(0); //Empty list. Exception e = Assert.assertThrows(Exception.class, - () -> MerkleTree.getInstance().createTree(hashList)); + () -> MerkleTree.build(hashList)); Assert.assertTrue(e instanceof IndexOutOfBoundsException); } @@ -117,7 +111,7 @@ public void test0HashNum() { */ public void test1HashNum() { List hashList = getHash(1); - MerkleTree tree = MerkleTree.getInstance().createTree(hashList); + MerkleTree tree = MerkleTree.build(hashList); Leaf root = tree.getRoot(); Assert.assertEquals(root.getHash(), hashList.get(0)); @@ -140,7 +134,7 @@ public void test1HashNum() { */ public void test2HashNum() { List hashList = getHash(2); - MerkleTree tree = MerkleTree.getInstance().createTree(hashList); + MerkleTree tree = MerkleTree.build(hashList); Leaf root = tree.getRoot(); Assert.assertEquals(root.getHash(), computeHash(hashList.get(0), hashList.get(1))); @@ -178,14 +172,13 @@ public void testAnyHashNum() { for (int hashNum = 1; hashNum <= maxNum; hashNum++) { int maxRank = getRank(hashNum); List hashList = getHash(hashNum); - MerkleTree tree = MerkleTree.getInstance().createTree(hashList); + MerkleTree tree = MerkleTree.build(hashList); Leaf root = tree.getRoot(); pareTree(root, hashList, maxRank, 0, 0); } } @Test - @Ignore public void testConcurrent() { Sha256Hash root1 = Sha256Hash.wrap( ByteString.fromHex("6cb38b4f493db8bacf26123cd4253bbfc530c708b97b3747e782f64097c3c482")); @@ -197,14 +190,16 @@ public void testConcurrent() { List list2 = IntStream.range(0, 10000).mapToObj(i -> Sha256Hash.of(true, ("byte2-" + i).getBytes(StandardCharsets.UTF_8))) .collect(Collectors.toList()); - Assert.assertEquals(root1, MerkleTree.getInstance().createTree(list1).getRoot().getHash()); - Assert.assertEquals(root2, MerkleTree.getInstance().createTree(list2).getRoot().getHash()); + Assert.assertEquals(root1, MerkleTree.build(list1).getRoot().getHash()); + Assert.assertEquals(root2, MerkleTree.build(list2).getRoot().getHash()); Assert.assertEquals(root1, MerkleRoot.root(list1)); Assert.assertEquals(root2, MerkleRoot.root(list2)); - exception.expect(ArrayIndexOutOfBoundsException.class); - IntStream.range(0, 1000).parallel().forEach(i -> Assert.assertEquals( - MerkleTree.getInstance().createTree(i % 2 == 0 ? list1 : list2).getRoot().getHash(), - MerkleRoot.root(i % 2 == 0 ? list1 : list2)) - ); + // MerkleTree.build is now per-instance with no shared state, so concurrent builds + // must yield correct roots without ArrayIndexOutOfBoundsException. + IntStream.range(0, 1000).parallel().forEach(i -> { + List list = i % 2 == 0 ? list1 : list2; + Sha256Hash expect = i % 2 == 0 ? root1 : root2; + Assert.assertEquals(expect, MerkleTree.build(list).getRoot().getHash()); + }); } }