diff --git a/core/src/main/java/com/google/bitcoin/core/Wallet.java b/core/src/main/java/com/google/bitcoin/core/Wallet.java index 2719da3a2..5edeaac29 100644 --- a/core/src/main/java/com/google/bitcoin/core/Wallet.java +++ b/core/src/main/java/com/google/bitcoin/core/Wallet.java @@ -19,7 +19,6 @@ package com.google.bitcoin.core; import com.google.bitcoin.core.TransactionConfidence.ConfidenceType; import com.google.bitcoin.core.WalletTransaction.Pool; import com.google.bitcoin.store.WalletProtobufSerializer; -import com.google.bitcoin.utils.EventListenerInvoker; import com.google.bitcoin.utils.Locks; import com.google.common.base.Objects; import com.google.common.base.Preconditions; @@ -293,14 +292,15 @@ public class Wallet implements Serializable, BlockChainListener { inactive = new HashMap(); pending = new HashMap(); dead = new HashMap(); + eventListeners = new CopyOnWriteArrayList(); createTransientState(); } private void createTransientState() { - eventListeners = new CopyOnWriteArrayList(); ignoreNextNewBlock = new HashSet(); txConfidenceListener = new TransactionConfidence.Listener() { public void onConfidenceChanged(Transaction tx) { + lock.lock(); invokeOnTransactionConfidenceChanged(tx); // Many onWalletChanged events will not occur because they are suppressed, eg, because: // - we are inside a re-org @@ -312,6 +312,7 @@ public class Wallet implements Serializable, BlockChainListener { // The latter case cannot happen today because we won't hear about it, but in future this may // become more common if conflict notices are implemented. invokeOnWalletChanged(); + lock.unlock(); } }; acceptTimeLockedTransactions = false; @@ -938,23 +939,6 @@ public class Wallet implements Serializable, BlockChainListener { } } - // Boilerplate that allows event listeners to delete themselves during execution, and auto locks the listener. - private void invokeOnCoinsReceived(final Transaction tx, final BigInteger balance, final BigInteger newBalance) { - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override public void invoke(WalletEventListener listener) { - listener.onCoinsReceived(Wallet.this, tx, balance, newBalance); - } - }); - } - - private void invokeOnCoinsSent(final Transaction tx, final BigInteger prevBalance, final BigInteger newBalance) { - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override public void invoke(WalletEventListener listener) { - listener.onCoinsSent(Wallet.this, tx, prevBalance, newBalance); - } - }); - } - /** *

Returns true if the given transaction sends coins to any of our keys, or has inputs spending any of our outputs, * and if includeDoubleSpending is true, also returns true if tx has inputs that are spending outputs which are @@ -1134,8 +1118,6 @@ public class Wallet implements Serializable, BlockChainListener { *

*

Used to update confidence data in each transaction and last seen block hash. Triggers auto saving. * Invokes the onWalletChanged event listener if there were any affected transactions.

- * - * @param block */ public void notifyNewBestBlock(StoredBlock block) throws VerificationException { // Check to see if this block has been seen before. @@ -1957,28 +1939,28 @@ public class Wallet implements Serializable, BlockChainListener { * in the list that was not already present. */ public int addKeys(final List keys) { + int added = 0; lock.lock(); try { // TODO: Consider making keys a sorted list or hashset so membership testing is faster. - int added = 0; for (final ECKey key : keys) { if (keychain.contains(key)) continue; keychain.add(key); - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override - public void invoke(WalletEventListener listener) { - listener.onKeyAdded(key); - } - }); added++; } if (autosaveToFile != null) { autoSave(); } - return added; } finally { lock.unlock(); } + for (ECKey key : keys) { + // TODO: Change this interface to be batch-oriented. + for (WalletEventListener listener : eventListeners) { + listener.onKeyAdded(key); + } + } + return added; } /** @@ -2429,14 +2411,8 @@ public class Wallet implements Serializable, BlockChainListener { } log.info("post-reorg balance is {}", Utils.bitcoinValueToFriendlyString(getBalance())); - // Inform event listeners that a re-org took place. They should save the wallet at this point. - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override - public void invoke(WalletEventListener listener) { - listener.onReorganize(Wallet.this); - } - }); + invokeOnReorganize(); onWalletChangedSuppressions--; invokeOnWalletChanged(); checkState(isConsistent()); @@ -2519,35 +2495,6 @@ public class Wallet implements Serializable, BlockChainListener { } } - private void invokeOnTransactionConfidenceChanged(final Transaction tx) { - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override - public void invoke(WalletEventListener listener) { - listener.onTransactionConfidenceChanged(Wallet.this, tx); - } - }); - } - - private int onWalletChangedSuppressions; - private void invokeOnWalletChanged() { - lock.lock(); - try { - // Don't invoke the callback in some circumstances, eg, whilst we are re-organizing or fiddling with - // transactions due to a new block arriving. It will be called later instead. - Preconditions.checkState(onWalletChangedSuppressions >= 0); - if (onWalletChangedSuppressions > 0) return; - // Call with the wallet locked. - EventListenerInvoker.invoke(eventListeners, new EventListenerInvoker() { - @Override - public void invoke(WalletEventListener listener) { - listener.onWalletChanged(Wallet.this); - } - }); - } finally { - lock.unlock(); - } - } - /** * Returns an immutable view of the transactions currently waiting for network confirmations. */ @@ -2712,4 +2659,73 @@ public class Wallet implements Serializable, BlockChainListener { lock.unlock(); } } + + ///////////////////////////////////////////////////////////////////////////////////////////////////////////////// + // + // Boilerplate for running event listeners - unlocks the wallet, runs, re-locks. + + private void invokeOnTransactionConfidenceChanged(Transaction tx) { + checkState(lock.isLocked()); + lock.unlock(); + try { + for (WalletEventListener listener : eventListeners) { + listener.onTransactionConfidenceChanged(this, tx); + } + } finally { + lock.lock(); + } + } + + private int onWalletChangedSuppressions = 0; + private void invokeOnWalletChanged() { + // Don't invoke the callback in some circumstances, eg, whilst we are re-organizing or fiddling with + // transactions due to a new block arriving. It will be called later instead. + checkState(lock.isLocked()); + Preconditions.checkState(onWalletChangedSuppressions >= 0); + if (onWalletChangedSuppressions > 0) return; + lock.unlock(); + try { + for (WalletEventListener listener : eventListeners) { + listener.onWalletChanged(this); + } + } finally { + lock.lock(); + } + } + + private void invokeOnCoinsReceived(Transaction tx, BigInteger balance, BigInteger newBalance) { + checkState(lock.isLocked()); + lock.unlock(); + try { + for (WalletEventListener listener : eventListeners) { + listener.onCoinsReceived(Wallet.this, tx, balance, newBalance); + } + } finally { + lock.lock(); + } + } + + private void invokeOnCoinsSent(Transaction tx, BigInteger prevBalance, BigInteger newBalance) { + checkState(lock.isLocked()); + lock.unlock(); + try { + for (WalletEventListener listener : eventListeners) { + listener.onCoinsSent(Wallet.this, tx, prevBalance, newBalance); + } + } finally { + lock.lock(); + } + } + + private void invokeOnReorganize() { + checkState(lock.isLocked()); + lock.unlock(); + try { + for (WalletEventListener listener : eventListeners) { + listener.onReorganize(Wallet.this); + } + } finally { + lock.lock(); + } + } } diff --git a/core/src/main/java/com/google/bitcoin/core/WalletEventListener.java b/core/src/main/java/com/google/bitcoin/core/WalletEventListener.java index 374b8d2b4..b311b01be 100644 --- a/core/src/main/java/com/google/bitcoin/core/WalletEventListener.java +++ b/core/src/main/java/com/google/bitcoin/core/WalletEventListener.java @@ -21,9 +21,6 @@ import java.math.BigInteger; /** *

Implementors are called when the contents of the wallet changes, for instance due to receiving/sending money * or a block chain re-organize. It may be convenient to derive from {@link AbstractWalletEventListener} instead.

- * - *

It is safe to call methods of the wallet during event listener execution, and also for a listener to remove itself. - * Other types of modifications generally aren't safe.

*/ public interface WalletEventListener { /** @@ -72,7 +69,6 @@ public interface WalletEventListener { */ void onReorganize(Wallet wallet); - // TODO: Flesh out the docs below some more to clarify what happens during re-orgs and other edge cases. /** *

Called on a Peer thread when a transaction changes its confidence level. You can also attach event listeners to * the individual transactions, if you don't care about all of them. Usually you would save the wallet to disk after @@ -113,9 +109,7 @@ public interface WalletEventListener { * *

When this is called you can refresh the UI contents from the wallet contents. It's more efficient to use * this rather than onTransactionConfidenceChanged() + onReorganize() because you only get one callback per block - * rather than one per transaction per block. Note that this is not called when a key is added. The wallet - * is locked whilst this handler is invoked, but if you relay the callback into another thread (eg the - * main UI thread) you should ensure to lock the wallet in the new thread as well.

+ * rather than one per transaction per block. Note that this is not called when a key is added.

*/ void onWalletChanged(Wallet wallet); diff --git a/examples/src/main/java/com/google/bitcoin/examples/PingService.java b/examples/src/main/java/com/google/bitcoin/examples/PingService.java index 8257f634f..0e9dff822 100644 --- a/examples/src/main/java/com/google/bitcoin/examples/PingService.java +++ b/examples/src/main/java/com/google/bitcoin/examples/PingService.java @@ -111,7 +111,7 @@ public class PingService { wallet.addEventListener(new AbstractWalletEventListener() { @Override public void onCoinsReceived(Wallet w, Transaction tx, BigInteger prevBalance, BigInteger newBalance) { - // Running on a peer thread. + // MUST BE THREAD SAFE assert !newBalance.equals(BigInteger.ZERO); if (!tx.isPending()) return; // It was broadcast, but we can't really verify it's valid until it appears in a block. diff --git a/examples/src/main/java/com/google/bitcoin/examples/RefreshWallet.java b/examples/src/main/java/com/google/bitcoin/examples/RefreshWallet.java index 48d3c4da4..c21cc5955 100644 --- a/examples/src/main/java/com/google/bitcoin/examples/RefreshWallet.java +++ b/examples/src/main/java/com/google/bitcoin/examples/RefreshWallet.java @@ -44,7 +44,7 @@ public class RefreshWallet { wallet.addEventListener(new AbstractWalletEventListener() { @Override - public void onCoinsReceived(Wallet w, Transaction tx, BigInteger prevBalance, BigInteger newBalance) { + public synchronized void onCoinsReceived(Wallet w, Transaction tx, BigInteger prevBalance, BigInteger newBalance) { System.out.println("\nReceived tx " + tx.getHashAsString()); System.out.println(tx.toString()); } diff --git a/examples/src/main/java/com/google/bitcoin/examples/toywallet/ToyWallet.java b/examples/src/main/java/com/google/bitcoin/examples/toywallet/ToyWallet.java index 61b7f8c23..2efa7cfa9 100644 --- a/examples/src/main/java/com/google/bitcoin/examples/toywallet/ToyWallet.java +++ b/examples/src/main/java/com/google/bitcoin/examples/toywallet/ToyWallet.java @@ -205,7 +205,7 @@ public class ToyWallet { wallet.addEventListener(new AbstractWalletEventListener() { @Override public void onWalletChanged(Wallet wallet) { - // This is running in some arbitrary bitcoinj provided thread with the wallet locked. + // MUST BE THREAD SAFE. final List txns = wallet.getTransactionsByTime(); SwingUtilities.invokeLater(new Runnable() { public void run() { diff --git a/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java b/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java index ad10471d7..988848c02 100644 --- a/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java +++ b/tools/src/main/java/com/google/bitcoin/tools/WalletTool.java @@ -503,7 +503,7 @@ public class WalletTool { } wallet.addEventListener(new AbstractWalletEventListener() { @Override - public void onChange() { + public synchronized void onChange() { super.onChange(); saveWallet(walletFile); BigInteger balance = wallet.getBalance(Wallet.BalanceType.ESTIMATED);