Fix singal disconnections in DiscoServiceWalker.

Fix how DiscoServiceWalker handles it disconnections from the discoInfoRequests and discoItemsRequest, as per swiften patch 'Fix memory leak warnings by Valgrind/LSAN' (b00c84574fc730eeeabb57df1f17b54855218193). Test-information: Unit tests pass ok. Change-Id: If47dca0f89822b0bf1da8dab1a3113a969f1c396
author: Alex Clayton <alex.clayton@isode.com> 2016-03-10 15:39:55 (GMT)
committer: Alex Clayton <alex.clayton@isode.com> 2016-03-14 14:44:26 (GMT)
commit: ecec6f30d6d91ea17b8ed9f6a31dc4702e759585 (patch)
tree: c549d0bdcdfe2515e40ad42082b1511598c6379e /src
parent: 9a5a18ee69e1daf50ac60c3c33619fa13743205f (diff)
download: stroke-ecec6f30d6d91ea17b8ed9f6a31dc4702e759585.zip
stroke-ecec6f30d6d91ea17b8ed9f6a31dc4702e759585.tar.bz2
1 files changed, 65 insertions, 39 deletions
diff --git a/src/com/isode/stroke/disco/DiscoServiceWalker.java b/src/com/isode/stroke/disco/DiscoServiceWalker.java
index 1b5c54e..39900aa 100644
--- a/src/com/isode/stroke/disco/DiscoServiceWalker.java
+++ b/src/com/isode/stroke/disco/DiscoServiceWalker.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010 Isode Limited.
+ * Copyright (c) 2010-2016 Isode Limited.
 * All rights reserved.
 * See the COPYING file for more information.
 */
@@ -23,9 +23,13 @@ import com.isode.stroke.signals.Slot2;
 import com.isode.stroke.signals.Signal2;
 import com.isode.stroke.signals.SignalConnection;
 import com.isode.stroke.base.NotNull;
 import java.util.logging.Logger;
+import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 import com.isode.stroke.base.NotNull;
 /**
@@ -46,8 +50,18 @@ public class DiscoServiceWalker {
        private SignalConnection onServiceFoundConnection;
        private SignalConnection onWalkAbortedConnection;
        private SignalConnection onWalkCompleteConnection;
-        private SignalConnection onResponseDiscoInfoConnection;
+        
-        private SignalConnection onResponseDiscoItemsConnection;
+        /**
+         * The discoInfo.onResponse connections for each GetDiscoInfoRequest
+         */
+        private Map<GetDiscoInfoRequest,SignalConnection> onResponseDiscoInfoConnections =
+                new HashMap<GetDiscoInfoRequest,SignalConnection>();
+        
+        /**
+         * The discoItems.onResponse connections for each GetDiscoItemsRequest
+         */
+        private Map<GetDiscoItemsRequest,SignalConnection> onResponseDiscoItemsConnections =
+                new HashMap<GetDiscoItemsRequest,SignalConnection>();
        /** Emitted for each service found. */
        public final Signal2<JID, DiscoInfo> onServiceFound = new Signal2<JID, DiscoInfo>();
@@ -99,17 +113,19 @@ public class DiscoServiceWalker {
         * End the walk.
         */
        public void endWalk() {
-                if (active_) {
+            if (active_) {
-                        logger_.fine("Ending walk to" + service_ + "\n");
+                logger_.fine("Ending walk to" + service_ + "\n");
-                        for (GetDiscoInfoRequest request : pendingDiscoInfoRequests_) {
+                for (SignalConnection discoInfoConnection : onResponseDiscoInfoConnections.values()) {
-                                onResponseDiscoInfoConnection.disconnect();
+                    discoInfoConnection.disconnect();
-                        }
+                }
-                        for (GetDiscoItemsRequest request : pendingDiscoItemsRequests_) {
+                onResponseDiscoInfoConnections.clear();
-                                onResponseDiscoItemsConnection.disconnect();
+                for (SignalConnection discoItemsConnection : onResponseDiscoItemsConnections.values()) {
-                        }
+                    discoItemsConnection.disconnect();
-                        active_ = false;
+                }
-                        onWalkAborted.emit();
+                onResponseDiscoItemsConnections.clear();
-                }               
+                active_ = false;
+                onWalkAborted.emit();
+            }           
        }
        public boolean isActive() {
@@ -121,13 +137,14 @@ public class DiscoServiceWalker {
                servicesBeingSearched_.add(jid);
                searchedServices_.add(jid);
                final GetDiscoInfoRequest discoInfoRequest = GetDiscoInfoRequest.create(jid, iqRouter_);
-                onResponseDiscoInfoConnection = discoInfoRequest.onResponse.connect(new Slot2<DiscoInfo, ErrorPayload>() {
+                SignalConnection connection = discoInfoRequest.onResponse.connect(new Slot2<DiscoInfo, ErrorPayload>() {
                        @Override
                        public void call(DiscoInfo info, ErrorPayload error) {
                                handleDiscoInfoResponse(info, error, discoInfoRequest);
                        }
                });
+                onResponseDiscoInfoConnections.put(discoInfoRequest, connection);
                pendingDiscoInfoRequests_.add(discoInfoRequest);
                discoInfoRequest.send();
        }
@@ -155,6 +172,10 @@ public class DiscoServiceWalker {
                logger_.fine("Disco info response from " + request.getReceiver() + "\n");
+                
+                SignalConnection connection = onResponseDiscoInfoConnections.remove(request);
+                connection.disconnect();
+                
                pendingDiscoInfoRequests_.remove(request);
                if (error != null) {
                        handleDiscoError(request.getReceiver(), error);
@@ -170,13 +191,14 @@ public class DiscoServiceWalker {
                boolean completed = false;
                if (couldContainServices) {
                        final GetDiscoItemsRequest discoItemsRequest = GetDiscoItemsRequest.create(request.getReceiver(), iqRouter_);
-                        onResponseDiscoItemsConnection = discoItemsRequest.onResponse.connect(new Slot2<DiscoItems, ErrorPayload>() {
+                        SignalConnection discoItemsConnection = discoItemsRequest.onResponse.connect(new Slot2<DiscoItems, ErrorPayload>() {
                                @Override
                                public void call(DiscoItems item, ErrorPayload error) {
                                        handleDiscoItemsResponse(item, error, discoItemsRequest);
                                }
                        });
+                        onResponseDiscoItemsConnections.put(discoItemsRequest, discoItemsConnection);
                        pendingDiscoItemsRequests_.add(discoItemsRequest);
                        discoItemsRequest.send();
                } else {
@@ -189,29 +211,33 @@ public class DiscoServiceWalker {
        }
        private void handleDiscoItemsResponse(DiscoItems items, ErrorPayload error, GetDiscoItemsRequest request) {
-                /* If we got canceled, don't do anything */
+            /* If we got canceled, don't do anything */
-                if (!active_) {
+            if (!active_) {
-                        return;
+                return;
-                }
+            }
-                logger_.fine("Received disco items from " + request.getReceiver() + "\n");
+            logger_.fine("Received disco items from " + request.getReceiver() + "\n");
-                pendingDiscoItemsRequests_.remove(request);
-                if (error != null) {
+            SignalConnection connection = onResponseDiscoItemsConnections.remove(request);
-                        handleDiscoError(request.getReceiver(), error);
+            connection.disconnect();
-                        return;
-                }
+            pendingDiscoItemsRequests_.remove(request);
-                for (DiscoItems.Item item : items.getItems()) {
+            if (error != null) {
-                        if (item.getNode().isEmpty()) {
+                handleDiscoError(request.getReceiver(), error);
-                                /* Don't look at noded items. It's possible that this will exclude some services,
+                return;
-                                 * but I've never seen one in the wild, and it's an easy fix for not looping.
+            }
-                                 */
+            for (DiscoItems.Item item : items.getItems()) {
-                                if(!searchedServices_.contains(item.getJID())) {
+                if (item.getNode().isEmpty()) {
-                                        logger_.fine("Received disco item " + item.getJID() + "\n");
+                    /* Don't look at noded items. It's possible that this will exclude some services,
-                                        walkNode(item.getJID());
+                     * but I've never seen one in the wild, and it's an easy fix for not looping.
-                                }
+                     */
-                        }
+                    if(!searchedServices_.contains(item.getJID())) {
-                }
+                        logger_.fine("Received disco item " + item.getJID() + "\n");
-                markNodeCompleted(request.getReceiver());
+                        walkNode(item.getJID());
+                    }
+                }
+            }
+            markNodeCompleted(request.getReceiver());
        }
        private void handleDiscoError(JID jid, ErrorPayload error) {
author	Alex Clayton <alex.clayton@isode.com>	2016-03-10 15:39:55 (GMT)
committer	Alex Clayton <alex.clayton@isode.com>	2016-03-14 14:44:26 (GMT)
commit	ecec6f30d6d91ea17b8ed9f6a31dc4702e759585 (patch)
tree	c549d0bdcdfe2515e40ad42082b1511598c6379e /src
parent	9a5a18ee69e1daf50ac60c3c33619fa13743205f (diff)
download	stroke-ecec6f30d6d91ea17b8ed9f6a31dc4702e759585.zip stroke-ecec6f30d6d91ea17b8ed9f6a31dc4702e759585.tar.bz2

diff --git a/src/com/isode/stroke/disco/DiscoServiceWalker.java b/src/com/isode/stroke/disco/DiscoServiceWalker.java index 1b5c54e..39900aa 100644 --- a/src/com/isode/stroke/disco/DiscoServiceWalker.java +++ b/src/com/isode/stroke/disco/DiscoServiceWalker.java
@@ -1,5 +1,5 @@
1	/*	1	/*
2	* Copyright (c) 2010 Isode Limited.	2	* Copyright (c) 2010-2016 Isode Limited.
3	* All rights reserved.	3	* All rights reserved.
4	* See the COPYING file for more information.	4	* See the COPYING file for more information.
5	*/	5	*/
@@ -23,9 +23,13 @@ import com.isode.stroke.signals.Slot2;
23	import com.isode.stroke.signals.Signal2;	23	import com.isode.stroke.signals.Signal2;
24	import com.isode.stroke.signals.SignalConnection;	24	import com.isode.stroke.signals.SignalConnection;
25	import com.isode.stroke.base.NotNull;	25	import com.isode.stroke.base.NotNull;
		26
26	import java.util.logging.Logger;	27	import java.util.logging.Logger;
		28	import java.util.HashMap;
27	import java.util.HashSet;	29	import java.util.HashSet;
		30	import java.util.Map;
28	import java.util.Set;	31	import java.util.Set;
		32
29	import com.isode.stroke.base.NotNull;	33	import com.isode.stroke.base.NotNull;
30		34
31	/**	35	/**
@@ -46,8 +50,18 @@ public class DiscoServiceWalker {
46	private SignalConnection onServiceFoundConnection;	50	private SignalConnection onServiceFoundConnection;
47	private SignalConnection onWalkAbortedConnection;	51	private SignalConnection onWalkAbortedConnection;
48	private SignalConnection onWalkCompleteConnection;	52	private SignalConnection onWalkCompleteConnection;
49	private SignalConnection onResponseDiscoInfoConnection;	53
50	private SignalConnection onResponseDiscoItemsConnection;	54	/**
		55	* The discoInfo.onResponse connections for each GetDiscoInfoRequest
		56	*/
		57	private Map<GetDiscoInfoRequest,SignalConnection> onResponseDiscoInfoConnections =
		58	new HashMap<GetDiscoInfoRequest,SignalConnection>();
		59
		60	/**
		61	* The discoItems.onResponse connections for each GetDiscoItemsRequest
		62	*/
		63	private Map<GetDiscoItemsRequest,SignalConnection> onResponseDiscoItemsConnections =
		64	new HashMap<GetDiscoItemsRequest,SignalConnection>();
51		65
52	/** Emitted for each service found. */	66	/** Emitted for each service found. */
53	public final Signal2<JID, DiscoInfo> onServiceFound = new Signal2<JID, DiscoInfo>();	67	public final Signal2<JID, DiscoInfo> onServiceFound = new Signal2<JID, DiscoInfo>();
@@ -99,17 +113,19 @@ public class DiscoServiceWalker {
99	* End the walk.	113	* End the walk.
100	*/	114	*/
101	public void endWalk() {	115	public void endWalk() {
102	if (active_) {	116	if (active_) {
103	logger_.fine("Ending walk to" + service_ + "\n");	117	logger_.fine("Ending walk to" + service_ + "\n");
104	for (GetDiscoInfoRequest request : pendingDiscoInfoRequests_) {	118	for (SignalConnection discoInfoConnection : onResponseDiscoInfoConnections.values()) {
105	onResponseDiscoInfoConnection.disconnect();	119	discoInfoConnection.disconnect();
106	}	120	}
107	for (GetDiscoItemsRequest request : pendingDiscoItemsRequests_) {	121	onResponseDiscoInfoConnections.clear();
108	onResponseDiscoItemsConnection.disconnect();	122	for (SignalConnection discoItemsConnection : onResponseDiscoItemsConnections.values()) {
109	}	123	discoItemsConnection.disconnect();
110	active_ = false;	124	}
111	onWalkAborted.emit();	125	onResponseDiscoItemsConnections.clear();
112	}	126	active_ = false;
		127	onWalkAborted.emit();
		128	}
113	}	129	}
114		130
115	public boolean isActive() {	131	public boolean isActive() {
@@ -121,13 +137,14 @@ public class DiscoServiceWalker {
121	servicesBeingSearched_.add(jid);	137	servicesBeingSearched_.add(jid);
122	searchedServices_.add(jid);	138	searchedServices_.add(jid);
123	final GetDiscoInfoRequest discoInfoRequest = GetDiscoInfoRequest.create(jid, iqRouter_);	139	final GetDiscoInfoRequest discoInfoRequest = GetDiscoInfoRequest.create(jid, iqRouter_);
124	onResponseDiscoInfoConnection = discoInfoRequest.onResponse.connect(new Slot2<DiscoInfo, ErrorPayload>() {	140	SignalConnection connection = discoInfoRequest.onResponse.connect(new Slot2<DiscoInfo, ErrorPayload>() {
125		141
126	@Override	142	@Override
127	public void call(DiscoInfo info, ErrorPayload error) {	143	public void call(DiscoInfo info, ErrorPayload error) {
128	handleDiscoInfoResponse(info, error, discoInfoRequest);	144	handleDiscoInfoResponse(info, error, discoInfoRequest);
129	}	145	}
130	});	146	});
		147	onResponseDiscoInfoConnections.put(discoInfoRequest, connection);
131	pendingDiscoInfoRequests_.add(discoInfoRequest);	148	pendingDiscoInfoRequests_.add(discoInfoRequest);
132	discoInfoRequest.send();	149	discoInfoRequest.send();
133	}	150	}
@@ -155,6 +172,10 @@ public class DiscoServiceWalker {
155		172
156	logger_.fine("Disco info response from " + request.getReceiver() + "\n");	173	logger_.fine("Disco info response from " + request.getReceiver() + "\n");
157		174
		175
		176	SignalConnection connection = onResponseDiscoInfoConnections.remove(request);
		177	connection.disconnect();
		178
158	pendingDiscoInfoRequests_.remove(request);	179	pendingDiscoInfoRequests_.remove(request);
159	if (error != null) {	180	if (error != null) {
160	handleDiscoError(request.getReceiver(), error);	181	handleDiscoError(request.getReceiver(), error);
@@ -170,13 +191,14 @@ public class DiscoServiceWalker {
170	boolean completed = false;	191	boolean completed = false;
171	if (couldContainServices) {	192	if (couldContainServices) {
172	final GetDiscoItemsRequest discoItemsRequest = GetDiscoItemsRequest.create(request.getReceiver(), iqRouter_);	193	final GetDiscoItemsRequest discoItemsRequest = GetDiscoItemsRequest.create(request.getReceiver(), iqRouter_);
173	onResponseDiscoItemsConnection = discoItemsRequest.onResponse.connect(new Slot2<DiscoItems, ErrorPayload>() {	194	SignalConnection discoItemsConnection = discoItemsRequest.onResponse.connect(new Slot2<DiscoItems, ErrorPayload>() {
174		195
175	@Override	196	@Override
176	public void call(DiscoItems item, ErrorPayload error) {	197	public void call(DiscoItems item, ErrorPayload error) {
177	handleDiscoItemsResponse(item, error, discoItemsRequest);	198	handleDiscoItemsResponse(item, error, discoItemsRequest);
178	}	199	}
179	});	200	});
		201	onResponseDiscoItemsConnections.put(discoItemsRequest, discoItemsConnection);
180	pendingDiscoItemsRequests_.add(discoItemsRequest);	202	pendingDiscoItemsRequests_.add(discoItemsRequest);
181	discoItemsRequest.send();	203	discoItemsRequest.send();
182	} else {	204	} else {
@@ -189,29 +211,33 @@ public class DiscoServiceWalker {
189	}	211	}
190		212
191	private void handleDiscoItemsResponse(DiscoItems items, ErrorPayload error, GetDiscoItemsRequest request) {	213	private void handleDiscoItemsResponse(DiscoItems items, ErrorPayload error, GetDiscoItemsRequest request) {
192	/* If we got canceled, don't do anything */	214	/* If we got canceled, don't do anything */
193	if (!active_) {	215	if (!active_) {
194	return;	216	return;
195	}	217	}
196		218
197	logger_.fine("Received disco items from " + request.getReceiver() + "\n");	219	logger_.fine("Received disco items from " + request.getReceiver() + "\n");
198	pendingDiscoItemsRequests_.remove(request);	220
199	if (error != null) {	221	SignalConnection connection = onResponseDiscoItemsConnections.remove(request);
200	handleDiscoError(request.getReceiver(), error);	222	connection.disconnect();
201	return;	223
202	}	224	pendingDiscoItemsRequests_.remove(request);
203	for (DiscoItems.Item item : items.getItems()) {	225	if (error != null) {
204	if (item.getNode().isEmpty()) {	226	handleDiscoError(request.getReceiver(), error);
205	/* Don't look at noded items. It's possible that this will exclude some services,	227	return;
206	* but I've never seen one in the wild, and it's an easy fix for not looping.	228	}
207	*/	229	for (DiscoItems.Item item : items.getItems()) {
208	if(!searchedServices_.contains(item.getJID())) {	230	if (item.getNode().isEmpty()) {
209	logger_.fine("Received disco item " + item.getJID() + "\n");	231	/* Don't look at noded items. It's possible that this will exclude some services,
210	walkNode(item.getJID());	232	* but I've never seen one in the wild, and it's an easy fix for not looping.
211	}	233	*/
212	}	234	if(!searchedServices_.contains(item.getJID())) {
213	}	235	logger_.fine("Received disco item " + item.getJID() + "\n");
214	markNodeCompleted(request.getReceiver());	236	walkNode(item.getJID());
		237	}
		238	}
		239	}
		240	markNodeCompleted(request.getReceiver());
215	}	241	}
216		242
217	private void handleDiscoError(JID jid, ErrorPayload error) {	243	private void handleDiscoError(JID jid, ErrorPayload error) {