From d264bc460cfd9065c085109b96fc510eaef4951a Mon Sep 17 00:00:00 2001 From: George Kiagiadakis <george.kiagiadakis@collabora.com> Date: Mon, 5 Apr 2021 15:15:22 +0300 Subject: [PATCH] registry: fix issues with dangling WpGlobal objects causing assertion failures The main problem observed is when a link that is owned by a WpLink is removed from the server because one of the linked nodes is gone. This would cause the APPEARS_ON_REGISTRY flag to go away but the WpGlobal would still remain in the globals list... To fix this, forcibly remove the global from the globals list when it is removed from the registry, even if it is still owned by some proxy. The proxy at that point is unable to function anyway, because we make sure to destroy the pw_proxy by removing FEATURE_BOUND when the global is removed from the registry. Additionally, ref global->proxy before removing FEATURE_BOUND to prevent crashing. If the proxy owns the global and the pw-proxy-destroyed signal causes whoever owns the proxy to drop his reference, _deactivate() will crash because no-one will be holding a ref to the proxy. --- lib/wp/object-manager.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/wp/object-manager.c b/lib/wp/object-manager.c index 84816c06..917ef4ff 100644 --- a/lib/wp/object-manager.c +++ b/lib/wp/object-manager.c @@ -1253,6 +1253,10 @@ wp_global_rm_flag (WpGlobal *global, guint rm_flag) if (!(global->flags & rm_flag)) return; + wp_trace_boxed (WP_TYPE_GLOBAL, global, + "remove global %u flag 0x%x [flags:0x%x, reg:%p]", + global->id, rm_flag, global->flags, reg); + /* global was owned by the proxy; by removing the flag, we clear out also the proxy pointer, which is presumably no longer valid and we notify all listeners that the proxy is gone */ @@ -1269,19 +1273,28 @@ wp_global_rm_flag (WpGlobal *global, guint rm_flag) /* destroy the proxy if it exists */ if (global->proxy) { + /* steal the proxy to avoid calling wp_registry_notify_rm_object() + again while removing OWNED_BY_PROXY; + keep a temporary ref so that _deactivate() doesn't crash in case the + pw-proxy-destroyed signal causes external references to be dropped */ + g_autoptr (WpGlobalProxy) proxy = + g_object_ref (g_steal_pointer (&global->proxy)); + + /* notify all listeners that the proxy is gone */ if (reg) - wp_registry_notify_rm_object (reg, global->proxy); - wp_object_deactivate (WP_OBJECT (global->proxy), WP_PROXY_FEATURE_BOUND); + wp_registry_notify_rm_object (reg, proxy); + + /* remove FEATURE_BOUND to destroy the underlying pw_proxy */ + wp_object_deactivate (WP_OBJECT (proxy), WP_PROXY_FEATURE_BOUND); /* if the proxy is not owning the global, unref it */ if (global->flags == 0) - g_object_unref (global->proxy); - global->proxy = NULL; + g_object_unref (proxy); } } - /* drop the registry's ref on global when it has no flags anymore */ - if (global->flags == 0 && reg) { + /* drop the registry's ref on global when it does not appear on the registry anymore */ + if (!(global->flags & WP_GLOBAL_FLAG_APPEARS_ON_REGISTRY) && reg) { g_clear_pointer (&g_ptr_array_index (reg->globals, global->id), wp_global_unref); } } -- GitLab