/pidgin/main: 50bb40c42245: facebook: implemented better HTTP co...

Changeset: 50bb40c42245618dc0a2cc92901905c701ad9f58
Author:	 James Geboski <[email protected]>
Date:	 2015-12-21 21:34 -0500
Branch:	 default
URL: https://hg.pidgin.im/pidgin/main/rev/50bb40c42245

Description:

facebook: implemented better HTTP connection handling

As of now, the Facebook prpl relies on purple_http_conn_cancel_all() to
cancel open HTTP connections when the PurpleConnection is closed. In
addition, purple_http_conn_is_cancelling() is used to determine when an
HTTP connection is forcibly closed. This is problematic as there are
times when a connection is both canceled and erroneous. And we need the
error to take precedence over the cancellation, or the prpl will simply
stall. Issues of such can be seen when the system is suspended and then
resumed with NetworkManager support built in.

This usage of purple_http_conn_cancel_all() leads to conflicts with the
two separate locations where HTTP connections are made. The connections
are only even canceled in the API interface, which is created by the
data interface, which also create HTTP connections. As a result, the
data interface may have open HTTP connections when the PurpleConnection
is closed. In turn this will cause unexpected behavior, non of which is
good.

The solution is track the connections locally in the prpl, and group
the connections in their respective interface. When an interface with
HTTP connections is destroyed, the connections are also closed. This
also gives the prpl an opportunity to differentiate between connections
which are canceled, and connections which are erroneous.

This removes the purple_http_conn_is_cancelling() function, which was
introduced in the patch (1cbcea2) that caused these regressions. This
function is likely of no use to anything else, and is also a not a good
idea for other things to use.

diffstat:

 libpurple/http.c                    |   7 ---
 libpurple/http.h                    |  10 ----
 libpurple/protocols/facebook/api.c  |  16 ++++---
 libpurple/protocols/facebook/data.c |  20 +++++++--
 libpurple/protocols/facebook/http.c |  74 ++++++++++++++++++++++++++++++++++
 libpurple/protocols/facebook/http.h |  79 +++++++++++++++++++++++++++++++++++++
 6 files changed, 177 insertions(+), 29 deletions(-)

diffs (truncated from 386 to 300 lines):

diff --git a/libpurple/http.c b/libpurple/http.c
--- a/libpurple/http.c
+++ b/libpurple/http.c
@@ -1748,13 +1748,6 @@ void purple_http_conn_cancel_all(PurpleC
 			"related to gc=%p (it shouldn't happen)\n", gc);
 }
 
-gboolean purple_http_conn_is_cancelling(PurpleHttpConnection *http_conn)
-{
-	if (http_conn == NULL)
-		return FALSE;
-	return http_conn->is_cancelling;
-}
-
 gboolean purple_http_conn_is_running(PurpleHttpConnection *http_conn)
 {
 	if (http_conn == NULL)
diff --git a/libpurple/http.h b/libpurple/http.h
--- a/libpurple/http.h
+++ b/libpurple/http.h
@@ -222,16 +222,6 @@ void purple_http_conn_cancel(PurpleHttpC
 void purple_http_conn_cancel_all(PurpleConnection *gc);
 
 /**
- * purple_http_conn_is_cancelling:
- * @http_conn: The HTTP connection (may be invalid pointer).
- *
- * Checks, if provided HTTP request is cancelling.
- *
- * Returns:          TRUE, if provided connection is currently cancelling.
- */
-gboolean purple_http_conn_is_cancelling(PurpleHttpConnection *http_conn);
-
-/**
  * purple_http_conn_is_running:
  * @http_conn: The HTTP connection (may be invalid pointer).
  *
diff --git a/libpurple/protocols/facebook/api.c b/libpurple/protocols/facebook/api.c
--- a/libpurple/protocols/facebook/api.c
+++ b/libpurple/protocols/facebook/api.c
@@ -50,8 +50,9 @@ enum
 
 struct _FbApiPrivate
 {
+	FbMqtt *mqtt;
+	FbHttpConns *cons;
 	PurpleConnection *gc;
-	FbMqtt *mqtt;
 	GHashTable *data;
 
 	FbId uid;
@@ -160,6 +161,7 @@ fb_api_dispose(GObject *obj)
 	FbApiPrivate *priv = FB_API(obj)->priv;
 	GHashTableIter iter;
 
+	fb_http_conns_cancel_all(priv->cons);
 	g_hash_table_iter_init(&iter, priv->data);
 
 	while (g_hash_table_iter_next(&iter, NULL, (gpointer) &fata)) {
@@ -167,14 +169,11 @@ fb_api_dispose(GObject *obj)
 		g_free(fata);
 	}
 
-	if (G_LIKELY(priv->gc != NULL)) {
-		purple_http_conn_cancel_all(priv->gc);
-	}
-
 	if (G_UNLIKELY(priv->mqtt != NULL)) {
 		g_object_unref(priv->mqtt);
 	}
 
+	fb_http_conns_free(priv->cons);
 	g_hash_table_destroy(priv->data);
 	g_hash_table_destroy(priv->mids);
 
@@ -485,6 +484,7 @@ fb_api_init(FbApi *api)
 	priv = G_TYPE_INSTANCE_GET_PRIVATE(api, FB_TYPE_API, FbApiPrivate);
 	api->priv = priv;
 
+	priv->cons = fb_http_conns_new();
 	priv->data = g_hash_table_new_full(g_direct_hash, g_direct_equal,
 	                                   NULL, NULL);
 	priv->mids = g_hash_table_new_full(g_int64_hash, g_int64_equal,
@@ -646,19 +646,20 @@ fb_api_http_chk(FbApi *api, PurpleHttpCo
 {
 	const gchar *data;
 	const gchar *msg;
+	FbApiPrivate *priv = api->priv;
 	gchar *emsg;
 	GError *err = NULL;
 	gint code;
 	gsize size;
 
-	if (G_UNLIKELY(purple_http_conn_is_cancelling(con))) {
-		/* Ignore canceling HTTP requests */
+	if (fb_http_conns_is_canceled(priv->cons)) {
 		return FALSE;
 	}
 
 	msg = purple_http_response_get_error(res);
 	code = purple_http_response_get_code(res);
 	data = purple_http_response_get_data(res, &size);
+	fb_http_conns_remove(priv->cons, con);
 
 	if (msg != NULL) {
 		emsg = g_strdup_printf("%s (%d)", msg, code);
@@ -755,6 +756,7 @@ fb_api_http_req(FbApi *api, const gchar 
 	data = fb_http_params_close(params, NULL);
 	purple_http_request_set_contents(req, data, -1);
 	ret = purple_http_request(priv->gc, req, callback, api);
+	fb_http_conns_add(priv->cons, ret);
 	purple_http_request_unref(req);
 
 	fb_util_debug(FB_UTIL_DEBUG_INFO, "HTTP Request (%p):", ret);
diff --git a/libpurple/protocols/facebook/data.c b/libpurple/protocols/facebook/data.c
--- a/libpurple/protocols/facebook/data.c
+++ b/libpurple/protocols/facebook/data.c
@@ -30,6 +30,7 @@
 struct _FbDataPrivate
 {
 	FbApi *api;
+	FbHttpConns *cons;
 	PurpleConnection *gc;
 	PurpleRoomlist *roomlist;
 	GQueue *msgs;
@@ -67,6 +68,7 @@ fb_data_dispose(GObject *obj)
 	GHashTableIter iter;
 	gpointer ptr;
 
+	fb_http_conns_cancel_all(priv->cons);
 	g_hash_table_iter_init(&iter, priv->evs);
 
 	while (g_hash_table_iter_next(&iter, NULL, &ptr)) {
@@ -77,6 +79,7 @@ fb_data_dispose(GObject *obj)
 		g_object_unref(priv->api);
 	}
 
+	fb_http_conns_free(priv->cons);
 	g_queue_free_full(priv->msgs, (GDestroyNotify) fb_api_message_free);
 
 	g_hash_table_destroy(priv->imgs);
@@ -101,7 +104,9 @@ fb_data_init(FbData *fata)
 	priv = G_TYPE_INSTANCE_GET_PRIVATE(fata, FB_TYPE_DATA, FbDataPrivate);
 	fata->priv = priv;
 
+	priv->cons = fb_http_conns_new();
 	priv->msgs = g_queue_new();
+
 	priv->imgs = g_hash_table_new_full(g_direct_hash, g_direct_equal,
 	                                   g_object_unref, NULL);
 	priv->unread = g_hash_table_new_full(fb_id_hash, fb_id_equal,
@@ -528,22 +533,25 @@ fb_data_image_cb(PurpleHttpConnection *c
 {
 	FbDataImage *img = data;
 	FbDataImagePrivate *priv = img->priv;
+	FbDataPrivate *driv = priv->fata->priv;
 	GError *err = NULL;
 
-	if (G_UNLIKELY(purple_http_conn_is_cancelling(con))) {
-		/* Ignore canceling HTTP requests */
+	if (fb_http_conns_is_canceled(driv->cons)) {
 		return;
 	}
 
+	fb_http_conns_remove(driv->cons, con);
 	fb_http_error_chk(res, &err);
+
 	priv->image = (guint8*) purple_http_response_get_data(res, &priv->size);
 	priv->func(img, err);
 
-	if (G_UNLIKELY(err != NULL)) {
+	if (G_LIKELY(err == NULL)) {
+		fb_data_image_queue(priv->fata);
+	} else {
 		g_error_free(err);
 	}
 
-	fb_data_image_queue(priv->fata);
 	g_object_unref(img);
 }
 
@@ -555,6 +563,7 @@ fb_data_image_queue(FbData *fata)
 	FbDataPrivate *priv;
 	GHashTableIter iter;
 	guint active = 0;
+	PurpleHttpConnection *con;
 
 	g_return_if_fail(FB_IS_DATA(fata));
 	priv = fata->priv;
@@ -579,7 +588,8 @@ fb_data_image_queue(FbData *fata)
 
 		img->priv->active = TRUE;
 		url = fb_data_image_get_url(img);
-		purple_http_get(priv->gc, fb_data_image_cb, img, url);
+		con = purple_http_get(priv->gc, fb_data_image_cb, img, url);
+		fb_http_conns_add(priv->cons, con);
 
 		if (++active >= FB_DATA_ICON_MAX) {
 			break;
diff --git a/libpurple/protocols/facebook/http.c b/libpurple/protocols/facebook/http.c
--- a/libpurple/protocols/facebook/http.c
+++ b/libpurple/protocols/facebook/http.c
@@ -25,6 +25,12 @@
 
 #include "http.h"
 
+struct _FbHttpConns
+{
+	GHashTable *cons;
+	gboolean canceled;
+};
+
 GQuark
 fb_http_error_quark(void)
 {
@@ -37,6 +43,74 @@ fb_http_error_quark(void)
 	return q;
 }
 
+FbHttpConns *
+fb_http_conns_new(void)
+{
+	FbHttpConns *cons;
+
+	cons = g_new0(FbHttpConns, 1);
+	cons->cons = g_hash_table_new(g_direct_hash, g_direct_equal);
+	return cons;
+}
+
+void
+fb_http_conns_free(FbHttpConns *cons)
+{
+	g_return_if_fail(cons != NULL);
+
+	g_hash_table_destroy(cons->cons);
+	g_free(cons);
+}
+
+void
+fb_http_conns_cancel_all(FbHttpConns *cons)
+{
+	GHashTableIter iter;
+	gpointer con;
+
+	g_return_if_fail(cons != NULL);
+	g_return_if_fail(!cons->canceled);
+
+	cons->canceled = TRUE;
+	g_hash_table_iter_init(&iter, cons->cons);
+
+	while (g_hash_table_iter_next(&iter, &con, NULL)) {
+		g_hash_table_iter_remove(&iter);
+		purple_http_conn_cancel(con);
+	}
+}
+
+gboolean
+fb_http_conns_is_canceled(FbHttpConns *cons)
+{
+	g_return_val_if_fail(cons != NULL, TRUE);
+	return cons->canceled;
+}
+
+void
+fb_http_conns_add(FbHttpConns *cons, PurpleHttpConnection *con)
+{
+	g_return_if_fail(cons != NULL);
+	g_return_if_fail(!cons->canceled);
+	g_hash_table_replace(cons->cons, con, con);
+}
+
+void
+fb_http_conns_remove(FbHttpConns *cons, PurpleHttpConnection *con)
+{
+	g_return_if_fail(cons != NULL);
+	g_return_if_fail(!cons->canceled);
+	g_hash_table_remove(cons->cons, con);
+}
+
+void
+fb_http_conns_reset(FbHttpConns *cons)
+{
+	g_return_if_fail(cons != NULL);
+	cons->canceled = FALSE;
+	g_hash_table_remove_all(cons->cons);
+}
+
 gboolean
 fb_http_error_chk(PurpleHttpResponse *res, GError **error)
 {
diff --git a/libpurple/protocols/facebook/http.h b/libpurple/protocols/facebook/http.h
--- a/libpurple/protocols/facebook/http.h
+++ b/libpurple/protocols/facebook/http.h
@@ -43,6 +43,13 @@
 #define FB_HTTP_ERROR fb_http_error_quark()
 
 /**
+ * FbHttpConns:
+ *
+ * Represents a set of #PurpleHttpConnection.

_______________________________________________
Commits mailing list
[email protected]
https://pidgin.im/cgi-bin/mailman/listinfo/commits