From 081663f857092615ba274f54bd41c2d8361d5aef Mon Sep 17 00:00:00 2001
From: Mike Hearn <hearn@google.com>
Date: Mon, 11 Mar 2013 12:10:41 +0100
Subject: [PATCH] Remove event listener in PeerGroup.removeWallet() to avoid a
 memory leak. Resolves issue 344.

---
 .../com/google/bitcoin/core/PeerGroup.java    | 61 +++++++++++--------
 1 file changed, 34 insertions(+), 27 deletions(-)

diff --git a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
index 407d721a..2b7dce57 100644
--- a/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
+++ b/core/src/main/java/com/google/bitcoin/core/PeerGroup.java
@@ -112,9 +112,28 @@ public class PeerGroup extends AbstractIdleService {
     private long fastCatchupTimeSecs;
     private final CopyOnWriteArrayList<Wallet> wallets;
 
-    private AbstractPeerEventListener getDataListener;
+    // This event listener is added to every peer. It's here so when we announce transactions via an "inv", every
+    // peer can fetch them.
+    private AbstractPeerEventListener getDataListener = new AbstractPeerEventListener() {
+        @Override
+        public List<Message> getData(Peer peer, GetDataMessage m) {
+            return handleGetData(m);
+        }
+    };
+
     private ClientBootstrap bootstrap;
     private int minBroadcastConnections = 0;
+    private AbstractWalletEventListener walletEventListener = new AbstractWalletEventListener() {
+        @Override
+        public void onKeyAdded(ECKey key) {
+            lock.lock();
+            try {
+                recalculateFastCatchupAndFilter();
+            } finally {
+                lock.unlock();
+            }
+        }
+    };;
 
     private class PeerStartupListener implements Peer.PeerLifecycleListener {
         public void onPeerConnected(Peer peer) {
@@ -221,14 +240,6 @@ public class PeerGroup extends AbstractIdleService {
         channels = new DefaultChannelGroup();
         peerDiscoverers = new CopyOnWriteArraySet<PeerDiscovery>(); 
         peerEventListeners = new CopyOnWriteArrayList<PeerEventListener>();
-        // This event listener is added to every peer. It's here so when we announce transactions via an "inv", every
-        // peer can fetch them.
-        getDataListener = new AbstractPeerEventListener() {
-            @Override
-            public List<Message> getData(Peer peer, GetDataMessage m) {
-                return handleGetData(m);
-            }
-        };
     }
 
     /**
@@ -576,14 +587,19 @@ public class PeerGroup extends AbstractIdleService {
 
     /**
      * <p>Link the given wallet to this PeerGroup. This is used for three purposes:</p>
+     *
      * <ol>
      *   <li>So the wallet receives broadcast transactions.</li>
      *   <li>Announcing pending transactions that didn't get into the chain yet to our peers.</li>
      *   <li>Set the fast catchup time using {@link PeerGroup#setFastCatchupTimeSecs(long)}, to optimize chain
      *       download.</li>
      * </ol>
+     *
      * <p>Note that this should be done before chain download commences because if you add a wallet with keys earlier
      * than the current chain head, the relevant parts of the chain won't be redownloaded for you.</p>
+     *
+     * <p>The Wallet will have an event listener registered on it, so to avoid leaks remember to use
+     * {@link PeerGroup#removeWallet(Wallet)} on it if you wish to keep the Wallet but lose the PeerGroup.</p>
      */
     public void addWallet(Wallet wallet) {
         lock.lock();
@@ -597,17 +613,7 @@ public class PeerGroup extends AbstractIdleService {
             // if a key is added. Of course, by then we may have downloaded the chain already. Ideally adding keys would
             // automatically rewind the block chain and redownload the blocks to find transactions relevant to those keys,
             // all transparently and in the background. But we are a long way from that yet.
-            wallet.addEventListener(new AbstractWalletEventListener() {
-                @Override
-                public void onKeyAdded(ECKey key) {
-                    lock.lock();
-                    try {
-                        recalculateFastCatchupAndFilter();
-                    } finally {
-                        lock.unlock();
-                    }
-                }
-            });
+            wallet.addEventListener(walletEventListener);
             recalculateFastCatchupAndFilter();
             updateVersionMessageRelayTxesBeforeFilter(getVersionMessage());
         } finally {
@@ -615,6 +621,14 @@ public class PeerGroup extends AbstractIdleService {
         }
     }
 
+    /**
+     * Unlinks the given wallet so it no longer receives broadcast transactions or has its transactions announced.
+     */
+    public void removeWallet(Wallet wallet) {
+        wallets.remove(checkNotNull(wallet));
+        wallet.removeEventListener(walletEventListener);
+    }
+
     private void recalculateFastCatchupAndFilter() {
         checkState(lock.isLocked());
         // Fully verifying mode doesn't use this optimization (it can't as it needs to see all transactions).
@@ -666,13 +680,6 @@ public class PeerGroup extends AbstractIdleService {
         }
     }
 
-    /**
-     * Unlinks the given wallet so it no longer receives broadcast transactions or has its transactions announced.
-     */
-    public void removeWallet(Wallet wallet) {
-        wallets.remove(checkNotNull(wallet));
-    }
-
     /**
      * Returns the number of currently connected peers. To be informed when this count changes, register a 
      * {@link PeerEventListener} and use the onPeerConnected/onPeerDisconnected methods.