From 6813ff4e6918917f85b6d2dae13220c60639aab5 Mon Sep 17 00:00:00 2001 From: Mike Hearn Date: Sat, 24 Dec 2011 19:30:55 +0000 Subject: [PATCH] Remove memory usage optimization that was complicating things. --- .../bitcoin/core/TransactionConfidence.java | 17 ++++---- src/com/google/bitcoin/core/Wallet.java | 43 +++++++++++-------- .../bitcoin/core/WalletEventListener.java | 5 ++- .../google/bitcoin/examples/PingService.java | 8 ++-- 4 files changed, 44 insertions(+), 29 deletions(-) diff --git a/src/com/google/bitcoin/core/TransactionConfidence.java b/src/com/google/bitcoin/core/TransactionConfidence.java index 09271558..1e225170 100644 --- a/src/com/google/bitcoin/core/TransactionConfidence.java +++ b/src/com/google/bitcoin/core/TransactionConfidence.java @@ -65,7 +65,7 @@ public class TransactionConfidence implements Serializable { private Set broadcastBy; /** The Transaction that this confidence object is associated with. */ private Transaction transaction; - private transient ArrayList listeners; + private transient ArrayList listeners = new ArrayList(1); // TODO: The advice below is a mess. There should be block chain listeners, see issue 94. /** @@ -81,8 +81,6 @@ public class TransactionConfidence implements Serializable { public synchronized void addEventListener(Listener listener) { if (listener == null) throw new IllegalArgumentException("listener is null"); - if (listeners == null) - listeners = new ArrayList(1); listeners.add(listener); } @@ -90,8 +88,6 @@ public class TransactionConfidence implements Serializable { if (listener == null) throw new IllegalArgumentException("listener is null"); listeners.remove(listener); - if (listeners.size() == 0) - listeners = null; } /** Describes the state of the transaction in general terms. Properties can be read to learn specifics. */ @@ -130,7 +126,8 @@ public class TransactionConfidence implements Serializable { /** * A confidence listener is informed when the level of confidence in a transaction is updated by something, like * for example a {@link Wallet}. You can add listeners to update your user interface or manage your order tracking - * system when confidence levels pass a certain threshold. Note that confidence can go down as well as up. + * system when confidence levels pass a certain threshold. Note that confidence can go down as well as up.. + * During listener execution, it's safe to remove the current listener but not others. */ public interface Listener { public void onConfidenceChanged(Transaction tx); @@ -331,11 +328,15 @@ public class TransactionConfidence implements Serializable { } private void runListeners() { - if (listeners == null) return; - for (Listener l : listeners) { + for (int i = 0; i < listeners.size(); i++) { + Listener l = listeners.get(i); synchronized (l) { l.onConfidenceChanged(transaction); } + if (listeners.get(i) != l) { + // Listener removed itself. + i--; + } } } } diff --git a/src/com/google/bitcoin/core/Wallet.java b/src/com/google/bitcoin/core/Wallet.java index 07f0fc9d..bc032cda 100644 --- a/src/com/google/bitcoin/core/Wallet.java +++ b/src/com/google/bitcoin/core/Wallet.java @@ -270,10 +270,19 @@ public class Wallet implements Serializable { // Event listeners may re-enter so we cannot make assumptions about wallet state after this loop completes. BigInteger balance = getBalance(); BigInteger newBalance = balance.add(value); - for (WalletEventListener l : eventListeners) { + invokeOnCoinsReceived(tx, balance, newBalance); + } + + private void invokeOnCoinsReceived(Transaction tx, BigInteger balance, BigInteger newBalance) { + for (int i = 0; i < eventListeners.size(); i++) { + WalletEventListener l = eventListeners.get(i); synchronized (l) { l.onCoinsReceived(this, tx, balance, newBalance); } + if (eventListeners.get(i) != l) { + // Listener removed itself. + i--; + } } } @@ -409,11 +418,7 @@ public class Wallet implements Serializable { // // TODO: Decide whether to run the event listeners, if a tx confidence listener already modified the wallet. if (!reorg && bestChain && valueDifference.compareTo(BigInteger.ZERO) > 0 && wtx == null) { - for (WalletEventListener l : eventListeners) { - synchronized (l) { - l.onCoinsReceived(this, tx, prevBalance, getBalance()); - } - } + invokeOnCoinsReceived(tx, prevBalance, getBalance()); } } @@ -452,11 +457,7 @@ public class Wallet implements Serializable { dead.put(doubleSpend.getHash(), doubleSpend); // Inform the event listeners of the newly dead tx. doubleSpend.getConfidence().setOverridingTransaction(tx); - for (WalletEventListener listener : eventListeners) { - synchronized (listener) { - listener.onDeadTransaction(this, doubleSpend, tx); - } - } + invokeOnDeadTransaction(doubleSpend, tx); } } @@ -1181,12 +1182,7 @@ public class Wallet implements Serializable { Transaction replacement = doubleSpent.getSpentBy().getParentTransaction(); dead.put(tx.getHash(), tx); pending.remove(tx.getHash()); - // Inform the event listeners of the newly dead tx. - for (WalletEventListener listener : eventListeners) { - synchronized (listener) { - listener.onDeadTransaction(this, tx, replacement); - } - } + invokeOnDeadTransaction(tx, replacement); break; } } @@ -1203,6 +1199,19 @@ public class Wallet implements Serializable { } } + private void invokeOnDeadTransaction(Transaction tx, Transaction replacement) { + for (int i = 0; i < eventListeners.size(); i++) { + WalletEventListener listener = eventListeners.get(i); + synchronized (listener) { + listener.onDeadTransaction(this, tx, replacement); + } + if (eventListeners.get(i) != listener) { + // Listener removed itself. + i--; + } + } + } + /** * Returns an immutable view of the transactions currently waiting for network confirmations. */ diff --git a/src/com/google/bitcoin/core/WalletEventListener.java b/src/com/google/bitcoin/core/WalletEventListener.java index 6bbeea63..9a87b475 100644 --- a/src/com/google/bitcoin/core/WalletEventListener.java +++ b/src/com/google/bitcoin/core/WalletEventListener.java @@ -22,7 +22,10 @@ import java.math.BigInteger; * Implementing WalletEventListener allows you to learn when the contents of the wallet changes due to * receiving money or a block chain re-organize. Methods are called with the event listener object locked so your * implementation does not have to be thread safe. It may be convenient to derive from - * {@link AbstractWalletEventListener} instead. + * {@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 { /** diff --git a/src/com/google/bitcoin/examples/PingService.java b/src/com/google/bitcoin/examples/PingService.java index 8cd36f38..13939779 100644 --- a/src/com/google/bitcoin/examples/PingService.java +++ b/src/com/google/bitcoin/examples/PingService.java @@ -115,6 +115,10 @@ public class PingService { public void onBlocksDownloaded(Peer peer, Block block, int blocksLeft) { super.onBlocksDownloaded(peer, block, blocksLeft); + // Don't bother printing during block chain downloads. + if (blocksLeft > 0) + return; + Set transactions = wallet.getTransactions(false, false); if (transactions.size() == 0) return; System.out.println("Confidences of wallet transactions:"); @@ -149,9 +153,7 @@ public class PingService { if (tx2.getConfidence().getConfidenceType() == TransactionConfidence.ConfidenceType.BUILDING) { // Coins were confirmed. bounceCoins(tx2); - - // TODO: Make this work. - // tx2.getConfidence().removeEventListener(this); + tx2.getConfidence().removeEventListener(this); } else { System.out.println(String.format("Confidence of %s changed, is now: %s", tx2.getHashAsString(), tx2.getConfidence().toString()));