From 4641535596d33c857bd09cb83698f251fc52bf16 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Tue, 10 Dec 2024 15:53:54 +0000 Subject: [PATCH] Revert "gdbus: Fix leak of method invocation when registering an object with closures" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 092fedd5f085a2f1966b5c34befe8b603c1a0f07. This was not the right change to make, and I shouldn’t have accepted the MR. The situation is laid out in this comment: https://gitlab.gnome.org/GNOME/glib/-/issues/2600#note_1385050 tl;dr: The reference on the `GDBusMethodInvocation` which is transferred in to the `GDBusInterfaceMethodCallFunc` is balanced by a reference transferred to `g_dbus_method_invocation_return_*()`. This is how the refcounting has always worked for these functions, and even if we’d probably arrange things differently if the code was written now, we can’t change those semantics without breaking API. In particular, bindings have various bits of custom code to account for these reference tranfers (since they can’t be represented using gobject-introspection annotations), so changing the semantics will break bindings. Fixes: #3559 --- gio/gdbusconnection.c | 2 +- gio/tests/gdbus-export.c | 19 +------------------ 2 files changed, 2 insertions(+), 19 deletions(-) diff --git a/gio/gdbusconnection.c b/gio/gdbusconnection.c index 3c82fd8de..0fac67009 100644 --- a/gio/gdbusconnection.c +++ b/gio/gdbusconnection.c @@ -5951,7 +5951,7 @@ register_with_closures_on_method_call (GDBusConnection *connection, g_value_set_variant (¶ms[5], parameters); g_value_init (¶ms[6], G_TYPE_DBUS_METHOD_INVOCATION); - g_value_take_object (¶ms[6], g_steal_pointer (&invocation)); + g_value_set_object (¶ms[6], invocation); g_closure_invoke (data->method_call_closure, NULL, G_N_ELEMENTS (params), params, NULL); diff --git a/gio/tests/gdbus-export.c b/gio/tests/gdbus-export.c index 5be560013..599df5bb5 100644 --- a/gio/tests/gdbus-export.c +++ b/gio/tests/gdbus-export.c @@ -161,23 +161,6 @@ foo_method_call (GDBusConnection *connection, } } -static void -foo_method_call_with_closure (GDBusConnection *connection, - const gchar *sender, - const gchar *object_path, - const gchar *interface_name, - const gchar *method_name, - GVariant *parameters, - GDBusMethodInvocation *invocation, - gpointer user_data) -{ - /* The call below takes ownership of the invocation but ownership is not - * passed into the callback so get an additional reference here */ - g_object_ref (invocation); - - foo_method_call (connection, sender, object_path, interface_name, method_name, parameters, invocation, user_data); -} - static GVariant * foo_get_property (GDBusConnection *connection, const gchar *sender, @@ -1457,7 +1440,7 @@ test_object_registration_with_closures (void) registration_id = g_dbus_connection_register_object_with_closures (c, "/foo/boss", (GDBusInterfaceInfo *) &foo_interface_info, - g_cclosure_new (G_CALLBACK (foo_method_call_with_closure), NULL, NULL), + g_cclosure_new (G_CALLBACK (foo_method_call), NULL, NULL), g_cclosure_new (G_CALLBACK (foo_get_property), NULL, NULL), g_cclosure_new (G_CALLBACK (foo_set_property), NULL, NULL), &error);