Changeset ea90275 for protocols/purple

Timestamp:

2016-11-13T20:10:17Z (8 years ago)

Author:

dequis <dx@…>

Branches:

master

Children:

9f03c47

Parents:

701ab812

git-author:

dequis <dx@…> (13-11-16 20:00:04)

git-committer:

dequis <dx@…> (13-11-16 20:10:17)

Message:

purple: fix file transfer memory management

This means cancelling transfers on logout to avoid crashes, keeping
track of timeouts, reffing and unreffing the xfers, listening to the
callbacks from UI and purple more carefully and using the correct
functions to free the correct things at the correct moments.

Originally intended to fix a crash triggered when the dcc stall timeout
kicks in after the account is offline, which is apparently very frequent
with telegram (it sends file transfers while fetching history, and
randomly disconnects a while later).

Trying to fix that meant opening a can of worms, but after three days of
work on this bug I'm pretty sure I've finished dealing with the
resulting mess and tested all the typical edge cases.

Location:

protocols/purple

Files:

• bpurple.h (modified) (1 diff)
• ft.c (modified) (10 diffs)
• purple.c (modified) (2 diffs)

protocols/purple/bpurple.h

@@ -14 +14 @@
     guint next_request_id;
     char *chat_list_server;
+    GSList *filetransfers;
 };
 

protocols/purple/ft.c

@@ -42 +42 @@
         char *fn, *handle;
         gboolean ui_wants_data;
+        int timeout;
 };
 
@@ -64 +65 @@
         struct prpl_xfer_data *px = ft->data;
 
-        purple_xfer_request_denied(px->xfer);
+        if (px->xfer) {
+                if (!purple_xfer_is_completed(px->xfer) && !purple_xfer_is_canceled(px->xfer)) {
+                        purple_xfer_cancel_local(px->xfer);
+                }
+                px->xfer->ui_data = NULL;
+                purple_xfer_unref(px->xfer);
+                px->xfer = NULL;
+        }
+}
+
+static void prpl_xfer_free(struct file_transfer *ft)
+{
+        struct prpl_xfer_data *px = ft->data;
+        struct purple_data *pd = px->ic->proto_data;
+
+        pd->filetransfers = g_slist_remove(pd->filetransfers, px);
+
+        if (px->xfer) {
+                px->xfer->ui_data = NULL;
+                purple_xfer_unref(px->xfer);
+        }
+
+        if (px->timeout) {
+                b_event_remove(px->timeout);
+        }
+
+        g_free(px->fn);
+        g_free(px->handle);
+        if (px->fd >= 0) {
+                close(px->fd);
+        }
+        g_free(px);
 }
 
 static void prplcb_xfer_new(PurpleXfer *xfer)
 {
+        purple_xfer_ref(xfer);
+
         if (purple_xfer_get_type(xfer) == PURPLE_XFER_RECEIVE) {
                 struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1);
+                struct purple_data *pd;
 
                 xfer->ui_data = px;
@@ -77 +112 @@
                 px->fd = -1;
                 px->ic = purple_ic_by_pa(xfer->account);
+
+                pd = px->ic->proto_data;
+                pd->filetransfers = g_slist_prepend(pd->filetransfers, px);
 
                 purple_xfer_set_local_filename(xfer, px->fn);
@@ -112 +150 @@
         px->ft->accept = prpl_xfer_accept;
         px->ft->canceled = prpl_xfer_canceled;
+        px->ft->free = prpl_xfer_free;
         px->ft->write_request = prpl_xfer_write_request;
 
@@ -164 +203 @@
 
 
-/* Generic (IM<>UI): */
 static void prplcb_xfer_destroy(PurpleXfer *xfer)
 {
         struct prpl_xfer_data *px = xfer->ui_data;
 
-        g_free(px->fn);
-        g_free(px->handle);
-        if (px->fd >= 0) {
-                close(px->fd);
-        }
-        g_free(px);
+        if (px) {
+                px->xfer = NULL;
+        }
 }
 
@@ -224 +259 @@
         struct prpl_xfer_data *px = xfer->ui_data;
 
-        if (px->ft) {
+        if (px && px->ft) {
                 imcb_file_canceled(px->ic, px->ft, "Canceled by remote end");
-        } else {
+        } else if (px) {
                 /* px->ft == NULL for sends, because of the two stages. :-/ */
                 imcb_error(px->ic, "File transfer cancelled by remote end");
@@ -240 +275 @@
 {
         struct prpl_xfer_data *px = g_new0(struct prpl_xfer_data, 1);
+        struct purple_data *pd;
         char *dir, *basename;
 
         ft->data = px;
         px->ft = ft;
+        px->ft->free = prpl_xfer_free;
 
         dir = g_strdup("/tmp/bitlbee-purple-ft.XXXXXX");
@@ -272 +309 @@
         px->handle = g_strdup(handle);
 
+        pd = px->ic->proto_data;
+        pd->filetransfers = g_slist_prepend(pd->filetransfers, px);
+
         imcb_log(ic,
                  "Due to libpurple limitations, the file has to be cached locally before proceeding with the actual file transfer. Please wait...");
 
-        b_timeout_add(0, purple_transfer_request_cb, ft);
+        px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft);
 }
 
@@ -295 +335 @@
         struct prpl_xfer_data *px = ft->data;
 
+        px->timeout = 0;
+
         if (ft->write == NULL) {
                 ft->write = prpl_xfer_write;
@@ -322 +364 @@
                 px->ft = NULL;
         } else {
-                b_timeout_add(0, purple_transfer_request_cb, ft);
+                px->timeout = b_timeout_add(0, purple_transfer_request_cb, ft);
         }
 
         return TRUE;
+}
+
+void purple_transfer_cancel_all(struct im_connection *ic)
+{
+        struct purple_data *pd = ic->proto_data;
+
+        while (pd->filetransfers) {
+                struct prpl_xfer_data *px = pd->filetransfers->data;
+
+                if (px->ft) {
+                        imcb_file_canceled(ic, px->ft, "Logging out");
+                }
+
+                pd->filetransfers = g_slist_remove(pd->filetransfers, px);
+        }
 }
 

protocols/purple/purple.c

@@ -43 +43 @@
                                    const char *message, const char *who);
 
+void purple_transfer_cancel_all(struct im_connection *ic);
+
 /* purple_request_input specific stuff */
 typedef void (*ri_callback_t)(gpointer, const gchar *);
@@ -383 +385 @@
         while (ic->groupchats) {
                 imcb_chat_free(ic->groupchats->data);
+        }
+
+        if (pd->filetransfers) {
+                purple_transfer_cancel_all(ic);
         }