TxConfidenceTable: Fix a lock in seen() and add a test.

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

core/src/main/java/org/bitcoinj/core/TxConfidenceTable.java

@@ -46,7 +46,8 @@ public class TxConfidenceTable {
             hash = confidence.getTransactionHash();
         }
     }
-    private LinkedHashMap<Sha256Hash, WeakConfidenceReference> table;
+    private final Map<Sha256Hash, WeakConfidenceReference> 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<Sha256Hash, WeakConfidenceReference>() {
             @Override
             protected boolean removeEldestEntry(Map.Entry<Sha256Hash, WeakConfidenceReference> 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 {

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.bitcoinj.core.Coin.COIN;
-import static org.junit.Assert.*;
+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.