Mercurial > pidgin

changeset 15440:56a2a0bb290a

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Fix a crash when a network_listen_range process is canceled before its UPnP port mapping completes, which occurs regularly on certain routers when file transfers are initiated and then quickly finished. Much thanks to Elliott Harris and Eric Richie for their hard work with me hunting this down and fixing it.
author Evan Schoenberg <evan.s@dreskin.net>
date Sun, 28 Jan 2007 15:05:23 +0000
parents a415922e2882
children 910f4be8fc73
files libpurple/network.c libpurple/upnp.c libpurple/upnp.h
diffstat 3 files changed, 87 insertions(+), 35 deletions(-) [+]
line wrap: on
line diff
--- a/libpurple/network.c	Sun Jan 28 01:24:15 2007 +0000
+++ b/libpurple/network.c	Sun Jan 28 15:05:23 2007 +0000
@@ -74,6 +74,7 @@
 	gboolean adding;
 	GaimNetworkListenCallback cb;
 	gpointer cb_data;
+	UPnPMappingAddRemove *mapping_data;
 };
 
 #ifdef HAVE_LIBNM
@@ -210,28 +211,29 @@
 		if (listen_data->retry) {
 			listen_data->retry = FALSE;
 			listen_data->adding = FALSE;
-			/* TODO: Need to keep track of this return value! */
-			gaim_upnp_remove_port_mapping(
-				gaim_network_get_port_from_fd(listen_data->listenfd),
-				(listen_data->socket_type == SOCK_STREAM) ? "TCP" : "UDP",
-				gaim_network_set_upnp_port_mapping_cb, listen_data);
+			listen_data->mapping_data = gaim_upnp_remove_port_mapping(
+						gaim_network_get_port_from_fd(listen_data->listenfd),
+						(listen_data->socket_type == SOCK_STREAM) ? "TCP" : "UDP",
+						gaim_network_set_upnp_port_mapping_cb, listen_data);
 			return;
 		}
 	} else if (!listen_data->adding) {
 		/* We've tried successfully to remove the port mapping.
 		 * Try to add it again */
 		listen_data->adding = TRUE;
-		/* TODO: Need to keep track of this return value! */
-		gaim_upnp_set_port_mapping(
-			gaim_network_get_port_from_fd(listen_data->listenfd),
-			(listen_data->socket_type == SOCK_STREAM) ? "TCP" : "UDP",
-			gaim_network_set_upnp_port_mapping_cb, listen_data);
+		listen_data->mapping_data = gaim_upnp_set_port_mapping(
+					gaim_network_get_port_from_fd(listen_data->listenfd),
+					(listen_data->socket_type == SOCK_STREAM) ? "TCP" : "UDP",
+					gaim_network_set_upnp_port_mapping_cb, listen_data);
 		return;
 	}
 
 	if (listen_data->cb)
 		listen_data->cb(listen_data->listenfd, listen_data->cb_data);
 
+	/* Clear the UPnP mapping data, since it's complete and gaim_netweork_listen_cancel() will try to cancel
+	 * it otherwise. */
+	listen_data->mapping_data = NULL;
 	gaim_network_listen_cancel(listen_data);
 }
 
@@ -327,11 +329,10 @@
 	listen_data->cb = cb;
 	listen_data->cb_data = cb_data;
 
-	/* TODO: Need to keep track of this return value! */
-	gaim_upnp_set_port_mapping(
-			gaim_network_get_port_from_fd(listenfd),
-			(socket_type == SOCK_STREAM) ? "TCP" : "UDP",
-			gaim_network_set_upnp_port_mapping_cb, listen_data);
+	listen_data->mapping_data = gaim_upnp_set_port_mapping(
+					gaim_network_get_port_from_fd(listenfd),
+					(socket_type == SOCK_STREAM) ? "TCP" : "UDP",
+					gaim_network_set_upnp_port_mapping_cb, listen_data);
 
 	return listen_data;
 }
@@ -370,6 +371,9 @@
 
 void gaim_network_listen_cancel(GaimNetworkListenData *listen_data)
 {
+	if (listen_data->mapping_data != NULL)
+		gaim_upnp_cancel_port_mapping(listen_data->mapping_data);
+
 	g_free(listen_data);
 }
 
--- a/libpurple/upnp.c	Sun Jan 28 01:24:15 2007 +0000
+++ b/libpurple/upnp.c	Sun Jan 28 15:05:23 2007 +0000
@@ -131,13 +131,16 @@
 	gchar *full_url;
 } UPnPDiscoveryData;
 
-typedef struct {
+struct _UPnPMappingAddRemove
+{
 	unsigned short portmap;
 	gchar protocol[4];
 	gboolean add;
 	GaimUPnPCallback cb;
 	gpointer cb_data;
-} UPnPMappingAddRemove;
+	guint tima; /* gaim_timeout_add handle */
+	GaimUtilFetchUrlData *gfud;
+};
 
 static GaimUPnPControlInfo control_info = {
 	GAIM_UPNP_STATUS_UNDISCOVERED,
@@ -666,12 +669,12 @@
 	gaim_upnp_discover_send_broadcast(dd);
 }
 
-static void
+static GaimUtilFetchUrlData*
 gaim_upnp_generate_action_message_and_send(const gchar* actionName,
 		const gchar* actionParams, GaimUtilFetchUrlCallback cb,
 		gpointer cb_data)
 {
-
+	GaimUtilFetchUrlData* gfud;
 	gchar* soapMessage;
 	gchar* totalSendMessage;
 	gchar* pathOfControl;
@@ -703,11 +706,13 @@
 	g_free(pathOfControl);
 	g_free(soapMessage);
 
-	gaim_util_fetch_url_request(control_info.control_url, FALSE, NULL, TRUE,
-			totalSendMessage, TRUE, cb, cb_data);
+	gfud = gaim_util_fetch_url_request(control_info.control_url, FALSE, NULL, TRUE,
+				totalSendMessage, TRUE, cb, cb_data);
 
 	g_free(totalSendMessage);
 	g_free(addressOfControl);
+	
+	return gfud;
 }
 
 const gchar *
@@ -884,8 +889,8 @@
 				ar->portmap, ar->protocol);
 		}
 
-		gaim_upnp_generate_action_message_and_send(action_name,
-				action_params, done_port_mapping_cb, ar);
+		ar->gfud = gaim_upnp_generate_action_message_and_send(action_name,
+						action_params, done_port_mapping_cb, ar);
 
 		g_free(action_params);
 		return;
@@ -904,7 +909,33 @@
 	return FALSE;
 }
 
-void
+void gaim_upnp_cancel_port_mapping(UPnPMappingAddRemove *ar)
+{
+	GSList *l;
+
+	/* Remove ar from discovery_callbacks if present; it was inserted after a cb.
+	 * The same cb may be in the list multple times, so be careful to remove the one assocaited with ar. */
+	l  = discovery_callbacks;
+	while (l)
+	{
+		if (l->next && (l->next->data == ar)) {
+			discovery_callbacks = g_slist_delete_link(discovery_callbacks, l->next);
+			discovery_callbacks = g_slist_delete_link(discovery_callbacks, l);
+		}
+
+		l = l->next;
+	}
+
+	if (ar->tima > 0)
+		gaim_timeout_remove(ar->tima);
+
+	if (ar->gfud)
+		gaim_util_fetch_url_cancel(ar->gfud);
+
+	g_free(ar);
+}
+
+UPnPMappingAddRemove *
 gaim_upnp_set_port_mapping(unsigned short portmap, const gchar* protocol,
 		GaimUPnPCallback cb, gpointer cb_data)
 {
@@ -925,7 +956,7 @@
 				discovery_callbacks, do_port_mapping_cb);
 		discovery_callbacks = g_slist_append(
 				discovery_callbacks, ar);
-		return;
+		return ar;
 	}
 
 	/* If we haven't had a successful UPnP discovery, check if 5 minutes has
@@ -934,22 +965,24 @@
 			(control_info.status == GAIM_UPNP_STATUS_UNABLE_TO_DISCOVER
 			 && (time(NULL) - control_info.lookup_time) > 300)) {
 		gaim_upnp_discover(do_port_mapping_cb, ar);
-		return;
+		return ar;
 	} else if(control_info.status == GAIM_UPNP_STATUS_UNABLE_TO_DISCOVER) {
 		if (cb) {
 			/* Asynchronously trigger a failed response */
-			gaim_timeout_add(10, fire_port_mapping_failure_cb, ar);
+			ar->tima = gaim_timeout_add(10, fire_port_mapping_failure_cb, ar);
 		} else {
 			/* No need to do anything if nobody expects a response*/
 			g_free(ar);
+			ar = NULL;
 		}
-		return;
+		return ar;
 	}
 
 	do_port_mapping_cb(TRUE, ar);
+	return ar;
 }
 
-void
+UPnPMappingAddRemove *
 gaim_upnp_remove_port_mapping(unsigned short portmap, const char* protocol,
 		GaimUPnPCallback cb, gpointer cb_data)
 {
@@ -968,7 +1001,7 @@
 				discovery_callbacks, do_port_mapping_cb);
 		discovery_callbacks = g_slist_append(
 				discovery_callbacks, ar);
-		return;
+		return ar;
 	}
 
 	/* If we haven't had a successful UPnP discovery, check if 5 minutes has
@@ -977,17 +1010,19 @@
 			(control_info.status == GAIM_UPNP_STATUS_UNABLE_TO_DISCOVER
 			 && (time(NULL) - control_info.lookup_time) > 300)) {
 		gaim_upnp_discover(do_port_mapping_cb, ar);
-		return;
+		return ar;
 	} else if(control_info.status == GAIM_UPNP_STATUS_UNABLE_TO_DISCOVER) {
 		if (cb) {
 			/* Asynchronously trigger a failed response */
-			gaim_timeout_add(10, fire_port_mapping_failure_cb, ar);
+			ar->tima = gaim_timeout_add(10, fire_port_mapping_failure_cb, ar);
 		} else {
 			/* No need to do anything if nobody expects a response*/
 			g_free(ar);
+			ar = NULL;
 		}
-		return;
+		return ar;
 	}
 
 	do_port_mapping_cb(TRUE, ar);
+	return ar;
 }
--- a/libpurple/upnp.h	Sun Jan 28 01:24:15 2007 +0000
+++ b/libpurple/upnp.h	Sun Jan 28 15:05:23 2007 +0000
@@ -26,6 +26,7 @@
 #ifndef _GAIM_UPNP_H_
 #define _GAIM_UPNP_H_
 
+typedef struct _UPnPMappingAddRemove UPnPMappingAddRemove;
 
 #ifdef __cplusplus
 extern "C" {
@@ -74,6 +75,14 @@
 const gchar* gaim_upnp_get_public_ip(void);
 
 /**
+ * Cancel a pending port mapping request initiated with either
+ * gaim_upnp_set_port_mapping() or gaim_upnp_remove_port_mapping().
+ *
+ * @param mapping_data The data returned when you initiated the UPnP mapping request.
+ */
+void gaim_upnp_cancel_port_mapping(UPnPMappingAddRemove *mapping_data);
+
+/**
  * Maps Ports in a UPnP enabled IGD that sits on the local network to
  * this gaim client. Essentially, this function takes care of the port
  * forwarding so things like file transfers can work behind NAT firewalls
@@ -83,8 +92,10 @@
  * @param cb an optional callback function to be notified when the mapping
  *           addition is complete
  * @param cb_data Extra data to be passed to the callback
+ *
+ * @return Data which can be passed to gaim_upnp_port_mapping_cancel() to cancel
  */
-void gaim_upnp_set_port_mapping(unsigned short portmap, const gchar* protocol,
+UPnPMappingAddRemove *gaim_upnp_set_port_mapping(unsigned short portmap, const gchar* protocol,
 		GaimUPnPCallback cb, gpointer cb_data);
 
 /**
@@ -98,8 +109,10 @@
  * @param cb an optional callback function to be notified when the mapping
  *           removal is complete
  * @param cb_data Extra data to be passed to the callback
+ *
+ * @return Data which can be passed to gaim_upnp_port_mapping_cancel() to cancel
  */
-void gaim_upnp_remove_port_mapping(unsigned short portmap,
+UPnPMappingAddRemove *gaim_upnp_remove_port_mapping(unsigned short portmap,
 		const gchar* protocol, GaimUPnPCallback cb, gpointer cb_data);
 
 /*@}*/