diff --git a/core/src/main/java/org/bitcoinj/core/PeerFilterProvider.java b/core/src/main/java/org/bitcoinj/core/PeerFilterProvider.java
index 3af57b77..7f77439f 100644
--- a/core/src/main/java/org/bitcoinj/core/PeerFilterProvider.java
+++ b/core/src/main/java/org/bitcoinj/core/PeerFilterProvider.java
@@ -31,6 +31,16 @@ public interface PeerFilterProvider {
*/
public long getEarliestKeyCreationTime();
+ /**
+ * Called on all registered filter providers before getBloomFilterElementCount and getBloomFilter are called.
+ * Once called, the provider should ensure that the items it will want to insert into the filter don't change.
+ * The reason is that all providers will have their element counts queried, and then a filter big enough for
+ * all of them will be specified. So the provider must use consistent state. There is guaranteed to be a matching
+ * call to endBloomFilterCalculation that can be used to e.g. unlock a lock.
+ */
+ public void beginBloomFilterCalculation();
+
+
/**
* Gets the number of elements that will be added to a bloom filter returned by
* {@link PeerFilterProvider#getBloomFilter(int, double, long)}
@@ -46,14 +56,5 @@ public interface PeerFilterProvider {
/** Whether this filter provider depends on the server updating the filter on all matches */
public boolean isRequiringUpdateAllBloomFilter();
- /**
- * Returns an object that will be locked before any other methods are called and unlocked afterwards. You must
- * provide one of these because the results from calling the above methods must be consistent. Otherwise it's
- * possible for the {@link org.bitcoinj.net.FilterMerger} to request the counts of a bunch of providers
- * with {@link #getBloomFilterElementCount()}, create a filter of the right size, call {@link #getBloomFilter(int, double, long)}
- * and then the filter provider discovers it's been mutated in the mean time and now has a different number of
- * elements. For instance, a Wallet that has keys added to it whilst a filter recalc is in progress could cause
- * experience this race.
- */
- public Lock getLock();
+ public void endBloomFilterCalculation();
}
diff --git a/core/src/main/java/org/bitcoinj/core/Wallet.java b/core/src/main/java/org/bitcoinj/core/Wallet.java
index 1ccc6428..5b15e184 100644
--- a/core/src/main/java/org/bitcoinj/core/Wallet.java
+++ b/core/src/main/java/org/bitcoinj/core/Wallet.java
@@ -3818,6 +3818,18 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
//region Bloom filtering
+ @Override
+ public void beginBloomFilterCalculation() {
+ lock.lock();
+ keychainLock.readLock().lock();
+ }
+
+ @Override
+ public void endBloomFilterCalculation() {
+ keychainLock.readLock().unlock();
+ lock.unlock();
+ }
+
/**
* Returns the number of distinct data items (note: NOT keys) that will be inserted into a bloom filter, when it
* is constructed.
@@ -4516,50 +4528,4 @@ public class Wallet extends BaseTaggableObject implements Serializable, BlockCha
}
}
//endregion
-
- /**
- * Returns the wallet lock under which most operations happen. This is here to satisfy the
- * {@link org.bitcoinj.core.PeerFilterProvider} interface and generally should not be used directly by apps.
- * In particular, do not hold this lock if you're display a send confirm screen to the user or for any other
- * long length of time, as it may cause processing holdups elsewhere. Instead, for the "confirm payment screen"
- * use case you should complete a candidate transaction, present it to the user (e.g. for fee purposes) and then
- * when they confirm - which may be quite some time later - recalculate the transaction and check if it's the same.
- * If not, redisplay the confirm window and try again.
- */
- @Override
- public Lock getLock() {
- return new Lock() {
- @Override
- public void lock() {
- lock.lock();
- keychainLock.readLock().lock();
- }
-
- @Override
- public void lockInterruptibly() throws InterruptedException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public boolean tryLock() {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public boolean tryLock(long l, TimeUnit unit) throws InterruptedException {
- throw new UnsupportedOperationException();
- }
-
- @Override
- public void unlock() {
- keychainLock.readLock().unlock();
- lock.unlock();
- }
-
- @Override
- public Condition newCondition() {
- throw new UnsupportedOperationException();
- }
- };
- }
}
diff --git a/core/src/main/java/org/bitcoinj/net/FilterMerger.java b/core/src/main/java/org/bitcoinj/net/FilterMerger.java
index d3dcb0c7..6732acf8 100644
--- a/core/src/main/java/org/bitcoinj/net/FilterMerger.java
+++ b/core/src/main/java/org/bitcoinj/net/FilterMerger.java
@@ -1,5 +1,6 @@
package org.bitcoinj.net;
+import com.google.common.collect.Lists;
import org.bitcoinj.core.BloomFilter;
import org.bitcoinj.core.PeerFilterProvider;
import com.google.common.collect.ImmutableList;
@@ -37,16 +38,15 @@ public class FilterMerger {
}
public Result calculate(ImmutableList providers) {
- LinkedList takenLocks = new LinkedList();
+ LinkedList begunProviders = Lists.newLinkedList();
try {
- // Lock all the providers so they cannot be mutated out from underneath us whilst we're in the process
- // of calculating the Bloom filter. All providers must be in a consistent, unchanging state because the
- // filter is a merged one that's large enough for all providers elements: if a provider were to get more
- // elements in the middle of the calculation, we might assert or calculate the filter wrongly.
+ // All providers must be in a consistent, unchanging state because the filter is a merged one that's
+ // large enough for all providers elements: if a provider were to get more elements in the middle of the
+ // calculation, we might assert or calculate the filter wrongly. Most providers use a lock here but
+ // snapshotting required state is also a legitimate strategy.
for (PeerFilterProvider provider : providers) {
- Lock lock = provider.getLock();
- lock.lock();
- takenLocks.add(lock);
+ provider.beginBloomFilterCalculation();
+ begunProviders.add(provider);
}
Result result = new Result();
result.earliestKeyTimeSecs = Long.MAX_VALUE;
@@ -79,8 +79,8 @@ public class FilterMerger {
result.earliestKeyTimeSecs -= 86400 * 7;
return result;
} finally {
- for (Lock takenLock : takenLocks) {
- takenLock.unlock();
+ for (PeerFilterProvider provider : begunProviders) {
+ provider.endBloomFilterCalculation();
}
}
}
diff --git a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java
index ab71c57e..59f3a233 100644
--- a/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java
+++ b/core/src/test/java/org/bitcoinj/core/BitcoindComparisonTool.java
@@ -121,8 +121,6 @@ public class BitcoindComparisonTool {
}
}, Threading.SAME_THREAD);
peers.addPeerFilterProvider(new PeerFilterProvider() {
- private final Lock lock = Threading.lock("pfp");
-
@Override public long getEarliestKeyCreationTime() {
return Long.MAX_VALUE;
}
@@ -137,8 +135,11 @@ public class BitcoindComparisonTool {
}
@Override
- public Lock getLock() {
- return lock;
+ public void beginBloomFilterCalculation() {
+ }
+
+ @Override
+ public void endBloomFilterCalculation() {
}
@Override public BloomFilter getBloomFilter(int size, double falsePositiveRate, long nTweak) {