qulogic/pidgin

facebook: implemented better HTTP connection handling

2015-12-21, James Geboski
50bb40c42245
Parents 1a155271da6c
Children 078fbf8d1a4e
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.
6 files changed, 177 insertions(+), 29 deletions(-)
  • +0 -7
    libpurple/http.c
  • +0 -10
    libpurple/http.h
  • +9 -7
    libpurple/protocols/facebook/api.c
  • +15 -5
    libpurple/protocols/facebook/data.c
  • +74 -0
    libpurple/protocols/facebook/http.c
  • +79 -0
    libpurple/protocols/facebook/http.h
    • --- a/libpurple/http.c Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/http.c Mon Dec 21 21:34:55 2015 -0500
    @@ -1748,13 +1748,6 @@
    "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)
    --- a/libpurple/http.h Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/http.h Mon Dec 21 21:34:55 2015 -0500
    @@ -222,16 +222,6 @@
    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).
    *
    --- a/libpurple/protocols/facebook/api.c Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/protocols/facebook/api.c Mon Dec 21 21:34:55 2015 -0500
    @@ -50,8 +50,9 @@
    struct _FbApiPrivate
    {
    + FbMqtt *mqtt;
    + FbHttpConns *cons;
    PurpleConnection *gc;
    - FbMqtt *mqtt;
    GHashTable *data;
    FbId uid;
    @@ -160,6 +161,7 @@
    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 @@
    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 @@
    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 @@
    {
    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 @@
    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);
    --- a/libpurple/protocols/facebook/data.c Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/protocols/facebook/data.c Mon Dec 21 21:34:55 2015 -0500
    @@ -30,6 +30,7 @@
    struct _FbDataPrivate
    {
    FbApi *api;
    + FbHttpConns *cons;
    PurpleConnection *gc;
    PurpleRoomlist *roomlist;
    GQueue *msgs;
    @@ -67,6 +68,7 @@
    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 @@
    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 @@
    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 @@
    {
    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 @@
    FbDataPrivate *priv;
    GHashTableIter iter;
    guint active = 0;
    + PurpleHttpConnection *con;
    g_return_if_fail(FB_IS_DATA(fata));
    priv = fata->priv;
    @@ -579,7 +588,8 @@
    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;
    --- a/libpurple/protocols/facebook/http.c Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/protocols/facebook/http.c Mon Dec 21 21:34:55 2015 -0500
    @@ -25,6 +25,12 @@
    #include "http.h"
    +struct _FbHttpConns
    +{
    + GHashTable *cons;
    + gboolean canceled;
    +};
    +
    GQuark
    fb_http_error_quark(void)
    {
    @@ -37,6 +43,74 @@
    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)
    {
    --- a/libpurple/protocols/facebook/http.h Mon Dec 21 21:08:54 2015 -0500
    +++ b/libpurple/protocols/facebook/http.h Mon Dec 21 21:34:55 2015 -0500
    @@ -43,6 +43,13 @@
    #define FB_HTTP_ERROR fb_http_error_quark()
    /**
    + * FbHttpConns:
    + *
    + * Represents a set of #PurpleHttpConnection.
    + */
    +typedef struct _FbHttpConns FbHttpConns;
    +
    +/**
    * FbHttpParams:
    *
    * Represents a set of key/value HTTP parameters.
    @@ -73,6 +80,78 @@
    fb_http_error_quark(void);
    /**
    + * fb_http_conns_new:
    + *
    + * Creates a new #FbHttpConns. The returned #FbHttpConns should be
    + * freed with #fb_http_conns_free() when no longer needed.
    + *
    + * Returns: The new #FbHttpConns.
    + */
    +FbHttpConns *
    +fb_http_conns_new(void);
    +
    +/**
    + * fb_http_conns_free:
    + * @cons: The #FbHttpConns.
    + *
    + * Frees all memory used by the #FbHttpConns. This will *not* cancel
    + * the any of the added #PurpleHttpConnection.
    + */
    +void
    +fb_http_conns_free(FbHttpConns *cons);
    +
    +/**
    + * fb_http_conns_cancel_all:
    + * @cons: The #FbHttpConns.
    + *
    + * Cancels each #PurpleHttpConnection in the #FbHttpConns.
    + */
    +void
    +fb_http_conns_cancel_all(FbHttpConns *cons);
    +
    +/**
    + * fb_http_conns_is_canceled:
    + * @cons: The #FbHttpConns.
    + *
    + * Determines if the #FbHttpConns has been canceled.
    + *
    + * Returns: #TRUE if it has been canceled, otherwise #FALSE.
    + */
    +gboolean
    +fb_http_conns_is_canceled(FbHttpConns *cons);
    +
    +/**
    + * fb_http_conns_add:
    + * @cons: The #FbHttpConns.
    + * @con: The #PurpleHttpConnection.
    + *
    + * Adds a #PurpleHttpConnection to the #FbHttpConns.
    + */
    +void
    +fb_http_conns_add(FbHttpConns *cons, PurpleHttpConnection *con);
    +
    +/**
    + * fb_http_conns_remove:
    + * @cons: The #FbHttpConns.
    + * @con: The #PurpleHttpConnection.
    + *
    + * Removes a #PurpleHttpConnection from the #FbHttpConns.
    + */
    +void
    +fb_http_conns_remove(FbHttpConns *cons, PurpleHttpConnection *con);
    +
    +/**
    + * fb_http_conns_reset:
    + * @cons: The #FbHttpConns.
    + *
    + * Resets the #FbHttpConns. This removes each #PurpleHttpConnection
    + * from the #FbHttpConns *without* canceling it. This allows the the
    + * #FbHttpConns to be reused.
    + */
    +void
    +fb_http_conns_reset(FbHttpConns *cons);
    +
    +/**
    * fb_http_error_chk:
    * @res: The #PurpleHttpResponse.
    * @error: The return location for the #GError or #NULL.