/pidgin/main: 92885e0456ed: facebook: fixed a crash with incomin...

[James Geboski]

Changeset: 92885e0456edad1a04d6dedb418e5d86bd7a3be6
Author:	 James Geboski <jgeboski at gmail.com>
Date:	 2016-01-25 13:04 -0500
Branch:	 default
URL: https://hg.pidgin.im/pidgin/main/rev/92885e0456ed

Description:

facebook: fixed a crash with incoming messages from non-friends

Currently, the plugin uses a single queue for incoming messages that
are being delayed. Messages are delayed for two reasons: the wait on
user information for unknown contact, or the wait on an image to get
fetched and stored. This allows the plugin to store the messages in the
proper order, but also ensure the messages are properly freed.

The problem with using a single queue for this is that each use case
treats the queue as if it is the only use case. This ends up leading to
the double freeing of message objects. In addition, the images do not
need to be queued as they are displayed in the order they are fetched,
rather than stalling the entire message queue.

This has been fixed by storing the image based message objects inside
of the image objects. This prevents from having to store the message
object in the message queue. The image objects are backed by a hash
table which provides the plugin a means of tracking the messages (as
well as the images). This ensures the messages are not leaked in the
event of an error.

This fixes startup crashes with unread messages from contacts who are
not friends with the local user. This issue is mostly seen with group
chats that have users who are not friends.

diffstat:

 libpurple/protocols/facebook/data.c     |   8 +++++++-
 libpurple/protocols/facebook/data.h     |   3 ++-
 libpurple/protocols/facebook/facebook.c |  12 ++++++------
 3 files changed, 15 insertions(+), 8 deletions(-)

diffs (102 lines):

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
@@ -45,6 +45,7 @@ struct _FbDataImagePrivate
 	gchar *url;
 	FbDataImageFunc func;
 	gpointer data;
+	GDestroyNotify dunc;
 
 	gboolean active;
 	const guint8 *image;
@@ -122,6 +123,10 @@ fb_data_image_dispose(GObject *obj)
 	FbDataImagePrivate *priv = img->priv;
 	FbData *fata = priv->fata;
 
+	if ((priv->dunc != NULL) && (priv->data != NULL)) {
+		priv->dunc(priv->data);
+	}
+
 	g_free(priv->url);
 	g_hash_table_steal(fata->priv->imgs, img);
 }
@@ -428,7 +433,7 @@ fb_data_take_messages(FbData *fata, FbId
 
 FbDataImage *
 fb_data_image_add(FbData *fata, const gchar *url, FbDataImageFunc func,
-                  gpointer data)
+                  gpointer data, GDestroyNotify dunc)
 {
 	FbDataImage *img;
 	FbDataImagePrivate *priv;
@@ -444,6 +449,7 @@ fb_data_image_add(FbData *fata, const gc
 	priv->url = g_strdup(url);
 	priv->func = func;
 	priv->data = data;
+	priv->dunc = dunc;
 
 	g_hash_table_insert(fata->priv->imgs, img, img);
 	return img;
diff --git a/libpurple/protocols/facebook/data.h b/libpurple/protocols/facebook/data.h
--- a/libpurple/protocols/facebook/data.h
+++ b/libpurple/protocols/facebook/data.h
@@ -306,6 +306,7 @@ fb_data_take_messages(FbData *fata, FbId
  * @url: The image URL.
  * @func: The #FbDataImageFunc.
  * @data: The user-defined data.
+ * @dunc: The #GDestroyNotify for @data or #NULL.
  *
  * Adds a new #FbDataImage to the #FbData. This is used to fetch images
  * from HTTP sources. After calling this, #fb_data_image_queue() should
@@ -315,7 +316,7 @@ fb_data_take_messages(FbData *fata, FbId
  */
 FbDataImage *
 fb_data_image_add(FbData *fata, const gchar *url, FbDataImageFunc func,
-                  gpointer data);
+                  gpointer data, GDestroyNotify dunc);
 
 /**
  * fb_data_image_get_active:
diff --git a/libpurple/protocols/facebook/facebook.c b/libpurple/protocols/facebook/facebook.c
--- a/libpurple/protocols/facebook/facebook.c
+++ b/libpurple/protocols/facebook/facebook.c
@@ -277,7 +277,8 @@ fb_cb_api_contacts(FbApi *api, GSList *u
 		csum = purple_buddy_icons_get_checksum_for_user(bdy);
 
 		if (!purple_strequal(csum, user->csum)) {
-			fb_data_image_add(fata, user->icon, fb_cb_icon, bdy);
+			fb_data_image_add(fata, user->icon, fb_cb_icon,
+			                  bdy, NULL);
 		}
 	}
 
@@ -431,7 +432,6 @@ fb_cb_image(FbDataImage *img, GError *er
 
 	fata = fb_data_image_get_fata(img);
 	msg = fb_data_image_get_data(img);
-	fb_data_remove_message(fata, msg);
 
 	if (G_UNLIKELY(error != NULL)) {
 		url = fb_data_image_get_url(img);
@@ -453,7 +453,7 @@ fb_cb_image(FbDataImage *img, GError *er
 
 	msgs = g_slist_prepend(msgs, msg);
 	fb_cb_api_messages(api, msgs, fata);
-	g_slist_free_full(msgs, (GDestroyNotify) fb_api_message_free);
+	g_slist_free(msgs);
 }
 
 static void
@@ -506,9 +506,9 @@ fb_cb_api_messages(FbApi *api, GSList *m
 		if (msg->flags & FB_API_MESSAGE_FLAG_IMAGE) {
 			if (!(msg->flags & FB_API_MESSAGE_FLAG_DONE)) {
 				msg = fb_api_message_dup(msg, TRUE);
-				fb_data_image_add(fata, msg->text,
-				                  fb_cb_image, msg);
-				fb_data_add_message(fata, msg);
+				fb_data_image_add(fata, msg->text, fb_cb_image,
+				                  msg, (GDestroyNotify)
+				                       fb_api_message_free);
 				fb_data_image_queue(fata);
 				continue;
 			}