[libsoup/sessionFeatureRef: 2/2] SoupSessionFeature: Keep features alive until all messages have been unqueued

[Claudio Saavedra <csaavedra src gnome org>]

commit cedfc390fd4d17b930b33f523b1f2c1d97e718c0
Author: Claudio Saavedra <csaavedra igalia com>
Date:   Fri Jan 18 17:32:58 2019 +0200

    SoupSessionFeature: Keep features alive until all messages have been unqueued
    
    Features can be removed at any point from a session, and if they are
    removed while there are messages in the queue, it is possible for
    callbacks connected to the message signals to be called after the feature
    has been disposed. Adding a reference in request_queue()
    and removing it in request_enqueue() ensures that the feature will not
    be disposed too early.
    
    Add a test case for the SoupCookieJar feature that reproduces this crash.
    
    Fixes #130

 libsoup/soup-session-feature.c | 26 ++++++++++++++------------
 tests/cookies-test.c           | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 12 deletions(-)
---
diff --git a/libsoup/soup-session-feature.c b/libsoup/soup-session-feature.c
index d8bb10a3..12716a62 100644
--- a/libsoup/soup-session-feature.c
+++ b/libsoup/soup-session-feature.c
@@ -66,8 +66,11 @@ request_queued (SoupSession *session, SoupMessage *msg, gpointer feature)
        if (soup_message_disables_feature (msg, feature))
                return;
 
-       SOUP_SESSION_FEATURE_GET_CLASS (feature)->
-               request_queued (feature, session, msg);
+       g_object_ref (feature);
+       if (SOUP_SESSION_FEATURE_GET_CLASS (feature)->request_queued) {
+               SOUP_SESSION_FEATURE_GET_CLASS (feature)->
+                       request_queued (feature, session, msg);
+       }
 }
 
 static void
@@ -87,8 +90,11 @@ request_unqueued (SoupSession *session, SoupMessage *msg, gpointer feature)
        if (soup_message_disables_feature (msg, feature))
                return;
 
-       SOUP_SESSION_FEATURE_GET_CLASS (feature)->
-               request_unqueued (feature, session, msg);
+       if (SOUP_SESSION_FEATURE_GET_CLASS (feature)->request_unqueued) {
+               SOUP_SESSION_FEATURE_GET_CLASS (feature)->
+                       request_unqueued (feature, session, msg);
+       }
+       g_object_unref (feature);
 }
 
 static void
@@ -97,20 +103,16 @@ soup_session_feature_real_attach (SoupSessionFeature *feature, SoupSession *sess
        g_object_weak_ref (G_OBJECT (session),
                           weak_notify_unref, g_object_ref (feature));
 
-       if (SOUP_SESSION_FEATURE_GET_CLASS (feature)->request_queued) {
-               g_signal_connect (session, "request_queued",
-                                 G_CALLBACK (request_queued), feature);
-       }
+       g_signal_connect (session, "request_queued",
+                         G_CALLBACK (request_queued), feature);
 
        if (SOUP_SESSION_FEATURE_GET_CLASS (feature)->request_started) {
                g_signal_connect (session, "request_started",
                                  G_CALLBACK (request_started), feature);
        }
 
-       if (SOUP_SESSION_FEATURE_GET_CLASS (feature)->request_unqueued) {
-               g_signal_connect (session, "request_unqueued",
-                                 G_CALLBACK (request_unqueued), feature);
-       }
+       g_signal_connect (session, "request_unqueued",
+                         G_CALLBACK (request_unqueued), feature);
 }
 
 void
diff --git a/tests/cookies-test.c b/tests/cookies-test.c
index e3f79cae..f2fcc63f 100644
--- a/tests/cookies-test.c
+++ b/tests/cookies-test.c
@@ -306,6 +306,38 @@ do_get_cookies_empty_host_test (void)
        soup_uri_free (uri);
 }
 
+static void
+send_callback (GObject *source_object,
+              GAsyncResult *res,
+              GMainLoop *loop)
+{
+       g_main_loop_quit (loop);
+}
+
+static void
+do_remove_feature_test (void)
+{
+       SoupSession *session;
+       SoupMessage *msg;
+       SoupURI *uri;
+       GMainLoop *loop;
+
+       session = soup_test_session_new (SOUP_TYPE_SESSION, NULL);
+       soup_session_add_feature_by_type (session, SOUP_TYPE_COOKIE_JAR);
+       uri = soup_uri_new_with_base (first_party_uri, "/index.html");
+       msg = soup_message_new_from_uri ("GET", uri);
+       soup_message_set_first_party (msg, first_party_uri);
+
+       loop = g_main_loop_new (NULL, TRUE);
+       soup_session_send_async (session, msg, NULL, (GAsyncReadyCallback)send_callback, loop);
+       soup_session_remove_feature_by_type (session, SOUP_TYPE_COOKIE_JAR);
+
+       g_main_loop_run(loop);
+
+       g_object_unref (msg);
+       soup_uri_free (uri);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -328,6 +360,7 @@ main (int argc, char **argv)
        g_test_add_func ("/cookies/parsing", do_cookies_parsing_test);
        g_test_add_func ("/cookies/parsing/no-path-null-origin", do_cookies_parsing_nopath_nullorigin);
        g_test_add_func ("/cookies/get-cookies/empty-host", do_get_cookies_empty_host_test);
+       g_test_add_func ("/cookies/remove-feature", do_remove_feature_test);
 
        ret = g_test_run ();