From 0c96402fc033c1fdf4f03826cdcfdb11330edea5 Mon Sep 17 00:00:00 2001 From: tau3 Date: Tue, 4 Sep 2018 15:29:59 +0200 Subject: [PATCH] TxConfidenceTable: Fix a lock in seen() and add a test. --- .../bitcoinj/core/TransactionConfidence.java | 5 +++ .../org/bitcoinj/core/TxConfidenceTable.java | 15 +++++-- .../bitcoinj/core/TxConfidenceTableTest.java | 45 ++++++++++++++++++- 3 files changed, 59 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/bitcoinj/core/TransactionConfidence.java b/core/src/main/java/org/bitcoinj/core/TransactionConfidence.java index 1f80cc53..f92bf982 100644 --- a/core/src/main/java/org/bitcoinj/core/TransactionConfidence.java +++ b/core/src/main/java/org/bitcoinj/core/TransactionConfidence.java @@ -62,6 +62,11 @@ import static com.google.common.base.Preconditions.*; * To make a copy that won't be changed, use {@link TransactionConfidence#duplicate()}. */ public class TransactionConfidence { + public static class Factory { + public TransactionConfidence createConfidence(Sha256Hash hash) { + return new TransactionConfidence(hash); + } + } /** * The peers that have announced the transaction to us. Network nodes don't have stable identities, so we use diff --git a/core/src/main/java/org/bitcoinj/core/TxConfidenceTable.java b/core/src/main/java/org/bitcoinj/core/TxConfidenceTable.java index 0552e5c9..f4748b4f 100644 --- a/core/src/main/java/org/bitcoinj/core/TxConfidenceTable.java +++ b/core/src/main/java/org/bitcoinj/core/TxConfidenceTable.java @@ -46,7 +46,8 @@ public class TxConfidenceTable { hash = confidence.getTransactionHash(); } } - private LinkedHashMap table; + private final Map table; + private final TransactionConfidence.Factory confidenceFactory; // This ReferenceQueue gets entries added to it when they are only weakly reachable, ie, the TxConfidenceTable is the // only thing that is tracking the confidence data anymore. We check it from time to time and delete table entries @@ -64,6 +65,10 @@ public class TxConfidenceTable { * @param size Max number of transactions to track. The table will fill up to this size then stop growing. */ public TxConfidenceTable(final int size) { + this(size, new TransactionConfidence.Factory()); + } + + TxConfidenceTable(final int size, TransactionConfidence.Factory confidenceFactory){ table = new LinkedHashMap() { @Override protected boolean removeEldestEntry(Map.Entry entry) { @@ -73,6 +78,7 @@ public class TxConfidenceTable { } }; referenceQueue = new ReferenceQueue<>(); + this.confidenceFactory = confidenceFactory; } /** @@ -139,12 +145,13 @@ public class TxConfidenceTable { TransactionConfidence confidence; boolean fresh = false; lock.lock(); - { + try { cleanTable(); confidence = getOrCreate(hash); fresh = confidence.markBroadcastBy(byPeer); + } finally { + lock.unlock(); } - lock.unlock(); if (fresh) confidence.queueListeners(TransactionConfidence.Listener.ChangeReason.SEEN_PEERS); return confidence; @@ -164,7 +171,7 @@ public class TxConfidenceTable { if (confidence != null) return confidence; } - TransactionConfidence newConfidence = new TransactionConfidence(hash); + TransactionConfidence newConfidence = confidenceFactory.createConfidence(hash); table.put(hash, new WeakConfidenceReference(newConfidence, referenceQueue)); return newConfidence; } finally { diff --git a/core/src/test/java/org/bitcoinj/core/TxConfidenceTableTest.java b/core/src/test/java/org/bitcoinj/core/TxConfidenceTableTest.java index 9417de68..585706e2 100644 --- a/core/src/test/java/org/bitcoinj/core/TxConfidenceTableTest.java +++ b/core/src/test/java/org/bitcoinj/core/TxConfidenceTableTest.java @@ -23,8 +23,15 @@ import org.junit.*; import java.net.*; -import static org.bitcoinj.core.Coin.*; -import static org.junit.Assert.*; +import static org.bitcoinj.core.Coin.COIN; +import static org.easymock.EasyMock.anyObject; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.expectLastCall; +import static org.easymock.EasyMock.replay; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; public class TxConfidenceTableTest { private static final NetworkParameters UNITTEST = UnitTestParams.get(); @@ -85,6 +92,40 @@ public class TxConfidenceTableTest { assertNull(run[0]); } + @Test + public void testSeen() { + PeerAddress peer = createMock(PeerAddress.class); + + Sha256Hash brokenHash = createMock(Sha256Hash.class); + Sha256Hash correctHash = createMock(Sha256Hash.class); + + TransactionConfidence brokenConfidence = createMock(TransactionConfidence.class); + expect(brokenConfidence.getTransactionHash()).andReturn(brokenHash); + expect(brokenConfidence.markBroadcastBy(peer)).andThrow(new ArithmeticException("some error")); + + TransactionConfidence correctConfidence = createMock(TransactionConfidence.class); + expect(correctConfidence.getTransactionHash()).andReturn(correctHash); + expect(correctConfidence.markBroadcastBy(peer)).andReturn(true); + correctConfidence.queueListeners(anyObject(TransactionConfidence.Listener.ChangeReason.class)); + expectLastCall(); + + TransactionConfidence.Factory factory = createMock(TransactionConfidence.Factory.class); + expect(factory.createConfidence(brokenHash)).andReturn(brokenConfidence); + expect(factory.createConfidence(correctHash)).andReturn(correctConfidence); + + replay(factory, brokenConfidence, correctConfidence); + + TxConfidenceTable table = new TxConfidenceTable(1, factory); + + try { + table.seen(brokenHash, peer); + } catch (ArithmeticException expected) { + // do nothing + } + + assertNotNull(table.seen(correctHash, peer)); + } + @Test public void invAndDownload() throws Exception { // Base case: we see a transaction announced twice and then download it. The count is in the confidence object.