From ae4d5553c1ca951800060615244c8b5553f3b807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 19:11:11 +0200 Subject: [PATCH 01/25] global: Use g_signal_emit to notify errors Shell global knows the signal ID, so we can just use it to emit the signal instead that looking up from the signal name. --- src/shell-global.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shell-global.c b/src/shell-global.c index 027c9d6c45..56e26ceaac 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -1341,7 +1341,7 @@ shell_global_notify_error (ShellGlobal *global, const char *msg, const char *details) { - g_signal_emit_by_name (global, "notify-error", msg, details); + g_signal_emit (global, shell_global_signals[NOTIFY_ERROR], 0, msg, details); } /** -- GitLab From 9c4c04780bb0aa1032519f9329641947af01db55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 18 Jul 2019 23:34:23 +0200 Subject: [PATCH 02/25] global: Initialize and stop the JS profiler together with the context The GjsProfiler needs to be initialized after the context and stopped before it. However, the context already stops the profiler during dispostion. So manage its lifetime together with the JS context, ensuring that we dispose the context when destroying it. --- src/main.c | 59 ---------------------------------------------- src/shell-global.c | 46 ++++++++++++++++++++++++++++++++++-- 2 files changed, 44 insertions(+), 61 deletions(-) diff --git a/src/main.c b/src/main.c index 5d07a43014..5b8d7ea68b 100644 --- a/src/main.c +++ b/src/main.c @@ -153,62 +153,6 @@ shell_fonts_init (void) cogl_pango_font_map_set_use_mipmapping (fontmap, FALSE); } -static void -shell_profiler_init (void) -{ - ShellGlobal *global; - GjsProfiler *profiler; - GjsContext *context; - const char *enabled; - const char *fd_str; - int fd = -1; - - /* Sysprof uses the "GJS_TRACE_FD=N" environment variable to connect GJS - * profiler data to the combined Sysprof capture. Since we are in control of - * the GjsContext, we need to proxy this FD across to the GJS profiler. - */ - - fd_str = g_getenv ("GJS_TRACE_FD"); - enabled = g_getenv ("GJS_ENABLE_PROFILER"); - if (fd_str == NULL || enabled == NULL) - return; - - global = shell_global_get (); - g_return_if_fail (global); - - context = _shell_global_get_gjs_context (global); - g_return_if_fail (context); - - profiler = gjs_context_get_profiler (context); - g_return_if_fail (profiler); - - if (fd_str) - { - fd = atoi (fd_str); - - if (fd > 2) - { - gjs_profiler_set_fd (profiler, fd); - gjs_profiler_start (profiler); - } - } -} - -static void -shell_profiler_shutdown (void) -{ - ShellGlobal *global; - GjsProfiler *profiler; - GjsContext *context; - - global = shell_global_get (); - context = _shell_global_get_gjs_context (global); - profiler = gjs_context_get_profiler (context); - - if (profiler) - gjs_profiler_stop (profiler); -} - static void malloc_statistics_callback (ShellPerfLog *perf_log, gpointer data) @@ -500,10 +444,7 @@ main (int argc, char **argv) dump_gjs_stack_on_signal (SIGSEGV); } - shell_profiler_init (); ecode = meta_run (); - shell_profiler_shutdown (); - g_debug ("Doing final cleanup"); _shell_global_destroy_gjs_context (shell_global_get ()); g_object_unref (shell_global_get ()); diff --git a/src/shell-global.c b/src/shell-global.c index 56e26ceaac..81bb3ae68f 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -334,6 +334,39 @@ switcheroo_vanished_cb (GDBusConnection *connection, g_object_notify (G_OBJECT (global), "switcheroo-control"); } +static void +shell_global_profiler_init (ShellGlobal *global) +{ + GjsProfiler *profiler; + const char *enabled; + const char *fd_str; + int fd = -1; + + /* Sysprof uses the "GJS_TRACE_FD=N" environment variable to connect GJS + * profiler data to the combined Sysprof capture. Since we are in control of + * the GjsContext, we need to proxy this FD across to the GJS profiler. + */ + + fd_str = g_getenv ("GJS_TRACE_FD"); + enabled = g_getenv ("GJS_ENABLE_PROFILER"); + if (fd_str == NULL || enabled == NULL) + return; + + profiler = gjs_context_get_profiler (global->js_context); + g_return_if_fail (profiler); + + if (fd_str) + { + fd = atoi (fd_str); + + if (fd > 2) + { + gjs_profiler_set_fd (profiler, fd); + gjs_profiler_start (profiler); + } + } +} + static void shell_global_init (ShellGlobal *global) { @@ -436,6 +469,15 @@ shell_global_init (ShellGlobal *global) switcheroo_vanished_cb, global, NULL); + + shell_global_profiler_init (global); +} + +static void +destroy_gjs_context (GjsContext *context) +{ + g_object_run_dispose (G_OBJECT (context)); + g_object_unref (context); } static void @@ -443,7 +485,7 @@ shell_global_finalize (GObject *object) { ShellGlobal *global = SHELL_GLOBAL (object); - g_clear_object (&global->js_context); + destroy_gjs_context (global->js_context); g_object_unref (global->settings); the_object = NULL; @@ -678,7 +720,7 @@ shell_global_get (void) void _shell_global_destroy_gjs_context (ShellGlobal *self) { - g_clear_object (&self->js_context); + g_clear_pointer (&self->js_context, destroy_gjs_context); } static guint32 -- GitLab From 2c5eeef86da4e8689230454919ae05805fc918ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 18 Jul 2019 23:54:41 +0200 Subject: [PATCH 03/25] global: Move objects unreffing to disposition and cleanup to finalization Unref global objects children during dispose phase, while destroy data in finalization. This will allow us to dispose the global object without unreffing it, but still clearing its contents. --- src/shell-global.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/shell-global.c b/src/shell-global.c index 81bb3ae68f..f292ec91fb 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -481,26 +481,34 @@ destroy_gjs_context (GjsContext *context) } static void -shell_global_finalize (GObject *object) +shell_global_dispose (GObject *object) { ShellGlobal *global = SHELL_GLOBAL (object); - destroy_gjs_context (global->js_context); - g_object_unref (global->settings); - - the_object = NULL; + g_clear_pointer (&global->js_context, destroy_gjs_context); g_cancellable_cancel (global->switcheroo_cancellable); g_clear_object (&global->switcheroo_cancellable); + g_clear_object (&global->settings); g_clear_object (&global->userdatadir_path); g_clear_object (&global->runtime_state_path); + G_OBJECT_CLASS (shell_global_parent_class)->dispose (object); +} + +static void +shell_global_finalize (GObject *object) +{ + ShellGlobal *global = SHELL_GLOBAL (object); + + the_object = NULL; + g_free (global->session_mode); g_free (global->imagedir); g_free (global->userdatadir); - g_hash_table_unref (global->save_ops); + g_hash_table_destroy (global->save_ops); G_OBJECT_CLASS(shell_global_parent_class)->finalize (object); } @@ -512,6 +520,7 @@ shell_global_class_init (ShellGlobalClass *klass) gobject_class->get_property = shell_global_get_property; gobject_class->set_property = shell_global_set_property; + gobject_class->dispose = shell_global_dispose; gobject_class->finalize = shell_global_finalize; shell_global_signals[NOTIFY_ERROR] = -- GitLab From 471d63b339f213704a939f828e7ed0dc8c9b9d58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 19 Jul 2019 18:47:09 +0200 Subject: [PATCH 04/25] global: Add destroy method breaking reference cycles through disposition The shell was destroying the JS Context using a specific method on exit, however since the context is now unreffed as part of the disposition, we can just force the disposition of ShellGlobal triggering the context finalization. Define _shell_global_destroy() private function where we force the disposition the global object and we unref it. --- src/main.c | 3 +-- src/shell-global-private.h | 2 +- src/shell-global.c | 14 ++++++++------ 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/main.c b/src/main.c index 5b8d7ea68b..9165d71872 100644 --- a/src/main.c +++ b/src/main.c @@ -446,8 +446,7 @@ main (int argc, char **argv) ecode = meta_run (); g_debug ("Doing final cleanup"); - _shell_global_destroy_gjs_context (shell_global_get ()); - g_object_unref (shell_global_get ()); + _shell_global_destroy (shell_global_get ()); return ecode; } diff --git a/src/shell-global-private.h b/src/shell-global-private.h index 9969691cb4..f8954ee14c 100644 --- a/src/shell-global-private.h +++ b/src/shell-global-private.h @@ -11,7 +11,7 @@ void _shell_global_init (const char *first_property_name, void _shell_global_set_plugin (ShellGlobal *global, MetaPlugin *plugin); -void _shell_global_destroy_gjs_context (ShellGlobal *global); +void _shell_global_destroy (ShellGlobal *global); GjsContext *_shell_global_get_gjs_context (ShellGlobal *global); diff --git a/src/shell-global.c b/src/shell-global.c index f292ec91fb..901fd4a4f9 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -719,17 +719,19 @@ shell_global_get (void) } /** - * _shell_global_destroy_gjs_context: (skip) + * _shell_global_destroy: (skip) * @self: global object * - * Destroys the GjsContext held by ShellGlobal, in order to break reference - * counting cycles. (The GjsContext holds a reference to ShellGlobal because - * it's available as window.global inside JS.) + * Destroys the #ShellGlobal, disposing child objects (such as the #GjsContext) + * in order to break reference counting cycles. + * The GjsContext holds a reference to ShellGlobal because it's available as + * window.global inside JS. */ void -_shell_global_destroy_gjs_context (ShellGlobal *self) +_shell_global_destroy (ShellGlobal *global) { - g_clear_pointer (&self->js_context, destroy_gjs_context); + g_object_run_dispose (G_OBJECT (global)); + g_object_unref (global); } static guint32 -- GitLab From 4ee16df81dea19155c9d3cb7f9f2c2f6c4f5a9a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 19 Jul 2019 22:56:39 +0200 Subject: [PATCH 05/25] main: Destroy and unref ShellGlobal after listing modes The JS context lifecycle is managed through the Shell global. So we can just destroy and unref it, instead of only unreffing the context. --- src/main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main.c b/src/main.c index 9165d71872..aa32745769 100644 --- a/src/main.c +++ b/src/main.c @@ -338,7 +338,8 @@ list_modes (const char *option_name, if (!gjs_context_eval (context, script, -1, "
", &status, NULL)) g_message ("Retrieving list of available modes failed."); - g_object_unref (context); + _shell_global_destroy (global); + exit (status); } -- GitLab From dc4a3afb15dc82b8b5ecbd1b5c2513fbfeca980c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sun, 21 Jul 2019 12:59:29 +0200 Subject: [PATCH 06/25] run-js-test: Destroy and unref ShellGlobal on exit The JS context lifecycle is managed through the Shell global. So we can just destroy it, instead of only unreffing the context. --- src/run-js-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/run-js-test.c b/src/run-js-test.c index ba5e875e87..3e8a35290f 100644 --- a/src/run-js-test.c +++ b/src/run-js-test.c @@ -112,7 +112,7 @@ main(int argc, char **argv) gjs_context_gc (js_context); gjs_context_gc (js_context); - g_object_unref (js_context); + _shell_global_destroy (global); g_free (script); exit (code); } -- GitLab From 8963fae1ebab4fc165656d0fd8185646c48d0575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sun, 21 Jul 2019 13:23:52 +0200 Subject: [PATCH 07/25] run-js-test: Use auto-pointers Avoid to explicitly free objects, strings and errors and make them to cleanup on function return. As per this, replace the exit calls with simply returns, otherwise the cleanup won't take place. --- src/run-js-test.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/run-js-test.c b/src/run-js-test.c index 3e8a35290f..b04f704fdd 100644 --- a/src/run-js-test.c +++ b/src/run-js-test.c @@ -46,13 +46,13 @@ static GOptionEntry entries[] = { int main(int argc, char **argv) { - GOptionContext *context; - GError *error = NULL; + g_autoptr (GError) error = NULL; + g_autofree char *script = NULL; + g_autofree char *title = NULL; ShellGlobal *global; + GOptionContext *context; GjsContext *js_context; - char *script; const char *filename; - char *title; gsize len; int code; @@ -62,8 +62,10 @@ main(int argc, char **argv) g_option_context_set_ignore_unknown_options (context, TRUE); g_option_context_add_main_entries (context, entries, NULL); - if (!g_option_context_parse (context, &argc, &argv, &error)) + if (!g_option_context_parse (context, &argc, &argv, &error)) { g_error ("option parsing failed: %s", error->message); + g_clear_error (&error); + } setlocale (LC_ALL, ""); @@ -76,7 +78,8 @@ main(int argc, char **argv) argc - 2, (const char**)argv + 2, &error)) { g_printerr ("Failed to defined ARGV: %s", error->message); - exit (1); + _shell_global_destroy (global); + return 1; } if (command != NULL) { @@ -88,31 +91,28 @@ main(int argc, char **argv) len = strlen (script); filename = ""; } else /*if (argc >= 2)*/ { - error = NULL; if (!g_file_get_contents (argv[1], &script, &len, &error)) { g_printerr ("%s\n", error->message); - exit (1); + _shell_global_destroy (global); + return 1; } filename = argv[1]; } title = g_filename_display_basename (filename); g_set_prgname (title); - g_free (title); /* evaluate the script */ error = NULL; if (!gjs_context_eval (js_context, script, len, filename, &code, &error)) { - g_free (script); g_printerr ("%s\n", error->message); - exit (1); } gjs_context_gc (js_context); gjs_context_gc (js_context); _shell_global_destroy (global); - g_free (script); - exit (code); + + return code; } -- GitLab From 9b436bb93b3158e366d0cbc6ab43bcf1bb2d359d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Sun, 21 Jul 2019 13:04:20 +0200 Subject: [PATCH 08/25] global: Make global constructor to be more standard _shell_global_init used to be the global object constructor, but was not returning it. Since we generally used it once initialized, we can just make this act like a proper constructor that returns the created object and that has the "new" suffix. Change this in both main and run-js-test. --- src/main.c | 8 ++++---- src/run-js-test.c | 3 +-- src/shell-global-private.h | 4 ++-- src/shell-global.c | 15 +++++++++------ 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/main.c b/src/main.c index aa32745769..7aaa6cea93 100644 --- a/src/main.c +++ b/src/main.c @@ -327,8 +327,7 @@ list_modes (const char *option_name, g_log_set_default_handler (shut_up, NULL); gtk_init_check (NULL, NULL); - _shell_global_init (NULL); - global = shell_global_get (); + global = _shell_global_new (NULL); context = _shell_global_get_gjs_context (global); shell_introspection_init (); @@ -386,6 +385,7 @@ main (int argc, char **argv) { GOptionContext *ctx; GError *error = NULL; + ShellGlobal *global; int ecode; bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); @@ -432,7 +432,7 @@ main (int argc, char **argv) if (session_mode == NULL) session_mode = is_gdm_mode ? (char *)"gdm" : (char *)"user"; - _shell_global_init ("session-mode", session_mode, NULL); + global = _shell_global_new ("session-mode", session_mode, NULL); dump_gjs_stack_on_signal (SIGABRT); dump_gjs_stack_on_signal (SIGFPE); @@ -447,7 +447,7 @@ main (int argc, char **argv) ecode = meta_run (); g_debug ("Doing final cleanup"); - _shell_global_destroy (shell_global_get ()); + _shell_global_destroy (global); return ecode; } diff --git a/src/run-js-test.c b/src/run-js-test.c index b04f704fdd..2c2dbef4a2 100644 --- a/src/run-js-test.c +++ b/src/run-js-test.c @@ -69,8 +69,7 @@ main(int argc, char **argv) setlocale (LC_ALL, ""); - _shell_global_init (NULL); - global = shell_global_get (); + global = _shell_global_new (NULL); js_context = _shell_global_get_gjs_context (global); /* prepare command line arguments */ diff --git a/src/shell-global-private.h b/src/shell-global-private.h index f8954ee14c..8e7ab69aec 100644 --- a/src/shell-global-private.h +++ b/src/shell-global-private.h @@ -6,8 +6,8 @@ #include -void _shell_global_init (const char *first_property_name, - ...); +ShellGlobal *_shell_global_new (const char *first_property_name, + ...); void _shell_global_set_plugin (ShellGlobal *global, MetaPlugin *plugin); diff --git a/src/shell-global.c b/src/shell-global.c index 901fd4a4f9..d1b6192d3f 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -675,12 +675,12 @@ shell_global_class_init (ShellGlobalClass *klass) } /* - * _shell_global_init: (skip) + * _shell_global_new: (skip) * @first_property_name: the name of the first property * @...: the value of the first property, followed optionally by more * name/value pairs, followed by %NULL * - * Initializes the shell global singleton with the construction-time + * Creates and initializes the shell global singleton with the construction-time * properties. * * There are currently no such properties, so @first_property_name should @@ -688,14 +688,16 @@ shell_global_class_init (ShellGlobalClass *klass) * * This call must be called before shell_global_get() and shouldn't be called * more than once. + * + * Return value: (transfer none): the singleton #ShellGlobal object */ -void -_shell_global_init (const char *first_property_name, - ...) +ShellGlobal * +_shell_global_new (const char *first_property_name, + ...) { va_list argument_list; - g_return_if_fail (the_object == NULL); + g_return_val_if_fail (the_object == NULL, the_object); va_start (argument_list, first_property_name); the_object = SHELL_GLOBAL (g_object_new_valist (SHELL_TYPE_GLOBAL, @@ -703,6 +705,7 @@ _shell_global_init (const char *first_property_name, argument_list)); va_end (argument_list); + return the_object; } /** -- GitLab From ca5682789c288fcd96091a861af69e493af01cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 18:30:37 +0200 Subject: [PATCH 09/25] global: Use auto-pointers to cleanup restart arguments Shell restart method might return earlier on failure, but in such cases we don't release initialized memory (such as the containing GPtrArray or the GError), so use auto-pointers to cleanup on return. https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/648 --- src/shell-global.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/shell-global.c b/src/shell-global.c index d1b6192d3f..3e15efa8ee 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -1290,14 +1290,14 @@ pre_exec_close_fds(void) void shell_global_reexec_self (ShellGlobal *global) { - GPtrArray *arr; + g_autoptr (GPtrArray) arr = NULL; gsize len; #if defined __linux__ || defined __sun - char *buf; + g_autofree char *buf = NULL; + g_autoptr (GError) error = NULL; char *buf_p; char *buf_end; - GError *error = NULL; if (!g_file_get_contents ("/proc/self/cmdline", &buf, &len, &error)) { @@ -1313,7 +1313,8 @@ shell_global_reexec_self (ShellGlobal *global) g_ptr_array_add (arr, NULL); #elif defined __OpenBSD__ - gchar **args, **args_p; + g_autoptr char **args = NULL; + gchar **args_p; gint mib[] = { CTL_KERN, KERN_PROC_ARGS, getpid(), KERN_PROC_ARGV }; if (sysctl (mib, G_N_ELEMENTS (mib), NULL, &len, NULL, 0) == -1) @@ -1323,7 +1324,6 @@ shell_global_reexec_self (ShellGlobal *global) if (sysctl (mib, G_N_ELEMENTS (mib), args, &len, NULL, 0) == -1) { g_warning ("failed to get command line args: %d", errno); - g_free (args); return; } @@ -1334,7 +1334,7 @@ shell_global_reexec_self (ShellGlobal *global) g_ptr_array_add (arr, NULL); #elif defined __FreeBSD__ - char *buf; + g_autoptr char *buf = NULL; char *buf_p; char *buf_end; gint mib[] = { CTL_KERN, KERN_PROC, KERN_PROC_ARGS, getpid() }; @@ -1346,7 +1346,6 @@ shell_global_reexec_self (ShellGlobal *global) if (sysctl (mib, G_N_ELEMENTS (mib), buf, &len, NULL, 0) == -1) { g_warning ("failed to get command line args: %d", errno); - g_free (buf); return; } @@ -1373,12 +1372,6 @@ shell_global_reexec_self (ShellGlobal *global) execvp (arr->pdata[0], (char**)arr->pdata); g_warning ("failed to reexec: %s", g_strerror (errno)); - g_ptr_array_free (arr, TRUE); -#if defined __linux__ || defined __FreeBSD__ - g_free (buf); -#elif defined __OpenBSD__ - g_free (args); -#endif } /** -- GitLab From 5127e2ca2efed40adeb7ed5da1f021907fef1ac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 16:04:52 +0200 Subject: [PATCH 10/25] global: Use connect object to connect to mutter signals ShellGlobal is connecting to stage display and meta-settings signals. Since the global object can be disposed before than the mutter objects, let's disconnect from them on global dispostion. https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/648 --- src/shell-global.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/shell-global.c b/src/shell-global.c index 3e15efa8ee..5f95e44144 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -1070,17 +1070,17 @@ _shell_global_set_plugin (ShellGlobal *global, st_entry_set_cursor_func (entry_cursor_func, global); st_clipboard_set_selection (meta_display_get_selection (display)); - g_signal_connect (global->stage, "notify::width", - G_CALLBACK (global_stage_notify_width), global); - g_signal_connect (global->stage, "notify::height", - G_CALLBACK (global_stage_notify_height), global); + g_signal_connect_object (global->stage, "notify::width", + G_CALLBACK (global_stage_notify_width), global, 0); + g_signal_connect_object (global->stage, "notify::height", + G_CALLBACK (global_stage_notify_height), global, 0); clutter_threads_add_repaint_func_full (CLUTTER_REPAINT_FLAGS_PRE_PAINT, global_stage_before_paint, global, NULL); - g_signal_connect (global->stage, "after-paint", - G_CALLBACK (global_stage_after_paint), global); + g_signal_connect_object (global->stage, "after-paint", + G_CALLBACK (global_stage_after_paint), global, 0); clutter_threads_add_repaint_func_full (CLUTTER_REPAINT_FLAGS_POST_PAINT, global_stage_after_swap, @@ -1099,10 +1099,10 @@ _shell_global_set_plugin (ShellGlobal *global, "End of frame, possibly including swap time", ""); - g_signal_connect (global->stage, "notify::key-focus", - G_CALLBACK (focus_actor_changed), global); - g_signal_connect (global->meta_display, "notify::focus-window", - G_CALLBACK (focus_window_changed), global); + g_signal_connect_object (global->stage, "notify::key-focus", + G_CALLBACK (focus_actor_changed), global, 0); + g_signal_connect_object (global->meta_display, "notify::focus-window", + G_CALLBACK (focus_window_changed), global, 0); if (global->xdisplay) g_signal_connect_object (global->meta_display, "x11-display-closing", @@ -1110,8 +1110,8 @@ _shell_global_set_plugin (ShellGlobal *global, backend = meta_get_backend (); settings = meta_backend_get_settings (backend); - g_signal_connect (settings, "ui-scaling-factor-changed", - G_CALLBACK (ui_scaling_factor_changed), global); + g_signal_connect_object (settings, "ui-scaling-factor-changed", + G_CALLBACK (ui_scaling_factor_changed), global, 0); global->focus_manager = st_focus_manager_get_for_stage (global->stage); -- GitLab From b831ac3aed6d6fd2f3e13981ec7976574dc1dba6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 19:34:36 +0200 Subject: [PATCH 11/25] global: Define ShellGlobalSingleton with destroy auto-pointer Use a private typedef ShellGlobalSingleton for ShellGlobal so that we can define a different auto-cleanup function for it and set it to _shell_global_destroy. In this way we can remove most of explicit destructions, but still making sure that we destroy the singleton that we initialize in main functions once we return. https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/648 --- src/main.c | 9 ++++----- src/run-js-test.c | 8 ++------ src/shell-global-private.h | 9 +++++++-- src/shell-global.c | 8 ++++---- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/main.c b/src/main.c index 7aaa6cea93..061582c71c 100644 --- a/src/main.c +++ b/src/main.c @@ -313,7 +313,7 @@ list_modes (const char *option_name, gpointer data, GError **error) { - ShellGlobal *global; + ShellGlobalSingleton *global; GjsContext *context; const char *script; int status; @@ -327,7 +327,7 @@ list_modes (const char *option_name, g_log_set_default_handler (shut_up, NULL); gtk_init_check (NULL, NULL); - global = _shell_global_new (NULL); + global = _shell_create_global_singleton (NULL); context = _shell_global_get_gjs_context (global); shell_introspection_init (); @@ -383,9 +383,9 @@ GOptionEntry gnome_shell_options[] = { int main (int argc, char **argv) { + g_autoptr (ShellGlobalSingleton) global = NULL; GOptionContext *ctx; GError *error = NULL; - ShellGlobal *global; int ecode; bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); @@ -432,7 +432,7 @@ main (int argc, char **argv) if (session_mode == NULL) session_mode = is_gdm_mode ? (char *)"gdm" : (char *)"user"; - global = _shell_global_new ("session-mode", session_mode, NULL); + global = _shell_create_global_singleton ("session-mode", session_mode, NULL); dump_gjs_stack_on_signal (SIGABRT); dump_gjs_stack_on_signal (SIGFPE); @@ -447,7 +447,6 @@ main (int argc, char **argv) ecode = meta_run (); g_debug ("Doing final cleanup"); - _shell_global_destroy (global); return ecode; } diff --git a/src/run-js-test.c b/src/run-js-test.c index 2c2dbef4a2..126c7c8428 100644 --- a/src/run-js-test.c +++ b/src/run-js-test.c @@ -46,10 +46,10 @@ static GOptionEntry entries[] = { int main(int argc, char **argv) { + g_autoptr (ShellGlobalSingleton) global = NULL; g_autoptr (GError) error = NULL; g_autofree char *script = NULL; g_autofree char *title = NULL; - ShellGlobal *global; GOptionContext *context; GjsContext *js_context; const char *filename; @@ -69,7 +69,7 @@ main(int argc, char **argv) setlocale (LC_ALL, ""); - global = _shell_global_new (NULL); + global = _shell_create_global_singleton (NULL); js_context = _shell_global_get_gjs_context (global); /* prepare command line arguments */ @@ -77,7 +77,6 @@ main(int argc, char **argv) argc - 2, (const char**)argv + 2, &error)) { g_printerr ("Failed to defined ARGV: %s", error->message); - _shell_global_destroy (global); return 1; } @@ -92,7 +91,6 @@ main(int argc, char **argv) } else /*if (argc >= 2)*/ { if (!g_file_get_contents (argv[1], &script, &len, &error)) { g_printerr ("%s\n", error->message); - _shell_global_destroy (global); return 1; } filename = argv[1]; @@ -111,7 +109,5 @@ main(int argc, char **argv) gjs_context_gc (js_context); gjs_context_gc (js_context); - _shell_global_destroy (global); - return code; } diff --git a/src/shell-global-private.h b/src/shell-global-private.h index 8e7ab69aec..3a39e6a83e 100644 --- a/src/shell-global-private.h +++ b/src/shell-global-private.h @@ -6,8 +6,11 @@ #include -ShellGlobal *_shell_global_new (const char *first_property_name, - ...); +typedef ShellGlobal ShellGlobalSingleton; + +ShellGlobalSingleton *_shell_create_global_singleton (const char *first_property_name, + ...); + void _shell_global_set_plugin (ShellGlobal *global, MetaPlugin *plugin); @@ -20,4 +23,6 @@ gboolean _shell_global_check_xdnd_event (ShellGlobal *global, void _shell_global_locate_pointer (ShellGlobal *global); +G_DEFINE_AUTOPTR_CLEANUP_FUNC (ShellGlobalSingleton, _shell_global_destroy) + #endif /* __SHELL_GLOBAL_PRIVATE_H__ */ diff --git a/src/shell-global.c b/src/shell-global.c index 5f95e44144..4ed84133c0 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -691,9 +691,9 @@ shell_global_class_init (ShellGlobalClass *klass) * * Return value: (transfer none): the singleton #ShellGlobal object */ -ShellGlobal * -_shell_global_new (const char *first_property_name, - ...) +ShellGlobalSingleton * +_shell_create_global_singleton (const char *first_property_name, + ...) { va_list argument_list; @@ -705,7 +705,7 @@ _shell_global_new (const char *first_property_name, argument_list)); va_end (argument_list); - return the_object; + return (ShellGlobalSingleton *) the_object; } /** -- GitLab From 9f88ad4414af55b4f320dca65f86844e53048ca2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 19:37:28 +0200 Subject: [PATCH 12/25] main: Use auto-pointers for variables in main function https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/648 --- src/main.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/main.c b/src/main.c index 061582c71c..c53d2f536c 100644 --- a/src/main.c +++ b/src/main.c @@ -384,8 +384,8 @@ int main (int argc, char **argv) { g_autoptr (ShellGlobalSingleton) global = NULL; - GOptionContext *ctx; - GError *error = NULL; + g_autoptr (GOptionContext) ctx = NULL; + g_autoptr (GError) error = NULL; int ecode; bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); @@ -400,11 +400,9 @@ main (int argc, char **argv) if (!g_option_context_parse (ctx, &argc, &argv, &error)) { g_printerr ("%s: %s\n", argv[0], error->message); - exit (1); + return 1; } - g_option_context_free (ctx); - meta_plugin_manager_set_plugin_type (gnome_shell_plugin_get_type ()); meta_set_wm_name (WM_NAME); -- GitLab From ca614da7bfdb3bd47d13c7940e4a9cfb7a585ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Fri, 26 Jul 2019 19:40:14 +0200 Subject: [PATCH 13/25] global: Emit new '::closing' signal before backend shutdown Emit a ::closing signal before destroying calling meta_finalize(), so that JS objects that need some specific cleanup can use the signal for doing it. --- src/main.c | 8 +++++--- src/shell-global-private.h | 2 ++ src/shell-global.c | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/main.c b/src/main.c index c53d2f536c..ca6d233c84 100644 --- a/src/main.c +++ b/src/main.c @@ -386,7 +386,6 @@ main (int argc, char **argv) g_autoptr (ShellGlobalSingleton) global = NULL; g_autoptr (GOptionContext) ctx = NULL; g_autoptr (GError) error = NULL; - int ecode; bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); bind_textdomain_codeset (GETTEXT_PACKAGE, "UTF-8"); @@ -443,8 +442,11 @@ main (int argc, char **argv) dump_gjs_stack_on_signal (SIGSEGV); } - ecode = meta_run (); + meta_start (); + meta_run_main_loop (); + shell_global_emit_closing (global); + meta_finalize (); g_debug ("Doing final cleanup"); - return ecode; + return meta_get_exit_code (); } diff --git a/src/shell-global-private.h b/src/shell-global-private.h index 3a39e6a83e..f993220488 100644 --- a/src/shell-global-private.h +++ b/src/shell-global-private.h @@ -23,6 +23,8 @@ gboolean _shell_global_check_xdnd_event (ShellGlobal *global, void _shell_global_locate_pointer (ShellGlobal *global); +void shell_global_emit_closing (ShellGlobal *global); + G_DEFINE_AUTOPTR_CLEANUP_FUNC (ShellGlobalSingleton, _shell_global_destroy) #endif /* __SHELL_GLOBAL_PRIVATE_H__ */ diff --git a/src/shell-global.c b/src/shell-global.c index 4ed84133c0..364c632b3a 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -120,6 +120,7 @@ enum { NOTIFY_ERROR, LOCATE_POINTER, + CLOSING, LAST_SIGNAL }; @@ -473,6 +474,12 @@ shell_global_init (ShellGlobal *global) shell_global_profiler_init (global); } +void +shell_global_emit_closing (ShellGlobal *global) +{ + g_signal_emit (the_object, shell_global_signals[CLOSING], 0); +} + static void destroy_gjs_context (GjsContext *context) { @@ -539,6 +546,13 @@ shell_global_class_init (ShellGlobalClass *klass) 0, NULL, NULL, NULL, G_TYPE_NONE, 0); + shell_global_signals[CLOSING] = + g_signal_new ("closing", + G_TYPE_FROM_CLASS (klass), + G_SIGNAL_RUN_LAST, + 0, + NULL, NULL, NULL, + G_TYPE_NONE, 0); g_object_class_install_property (gobject_class, PROP_SESSION_MODE, -- GitLab From 6a40510d04034dbe84b04c19baccc6b1573384f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 11:39:17 +0100 Subject: [PATCH 14/25] layout: Destroy uiGroup elements owned by gnome-shell on shutdown This avoids various warnings etc during tear down, by simply cleaning up on shutdown, instead of a side effect of the backend and stage destruction. We shouldn't destroy MetaWindowGroup actors though, they are not owned by gnome-shell but mutter, so leave those be returning them to their original position, i.e. directly on the stage. Use a "while not emty destroy first element" approach to destruction, as in some cases destroying an actor will destroy one or more siblings, causing an iterator based approach to fail. We also can't destroy the uiGroup actor alone, as it uses an iterator internally, which doesn't handle sibling destroying each other. --- js/ui/layout.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/js/ui/layout.js b/js/ui/layout.js index 70ece6cab8..03207a75b0 100644 --- a/js/ui/layout.js +++ b/js/ui/layout.js @@ -216,6 +216,18 @@ var LayoutManager = GObject.registerClass({ global.stage.add_child(this.uiGroup); + global.connect_after('closing', () => { + Main.uiGroup.remove_child(global.window_group); + global.stage.add_child(global.window_group); + + Main.uiGroup.remove_child(global.top_window_group); + global.stage.add_child(global.top_window_group); + + while (Main.uiGroup.get_children().length > 0) + Main.uiGroup.get_children()[0].destroy(); + Main.uiGroup.destroy(); + }); + global.stage.remove_actor(global.window_group); this.uiGroup.add_actor(global.window_group); -- GitLab From d5fcd9346c671417aaf54021a61d61f04bc672db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 09:11:14 +0100 Subject: [PATCH 15/25] iconGrid: Clean up on shutdown This avoids calling Javascript code during the last garbage collection sweep. --- js/ui/iconGrid.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/js/ui/iconGrid.js b/js/ui/iconGrid.js index 0272c0e233..c31d856db3 100644 --- a/js/ui/iconGrid.js +++ b/js/ui/iconGrid.js @@ -371,6 +371,15 @@ var IconGridLayout = GObject.registerClass({ this._iconSizeUpdateResolveCbs = []; this._childrenMaxSize = -1; + this._container = null; + + global.connect('closing', this._onClosing.bind(this)); + } + + _onClosing() { + for (const item of this._items.keys()) + this._unlinkItem(item); + this._clearContainer(); } _findBestIconSize() { @@ -722,9 +731,15 @@ var IconGridLayout = GObject.registerClass({ } } - vfunc_set_container(container) { - if (this._container) + _clearContainer() { + if (this._container) { this._container.disconnect(this._containerDestroyedId); + this._container = null; + } + } + + vfunc_set_container(container) { + this._clearContainer(); this._container = container; -- GitLab From eed9001194a8b98e01861ebf1028d7b61f019fee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 11:52:13 +0100 Subject: [PATCH 16/25] appDisplay: Handle swipe tracker being gone when unmapping This can happen when the app view was unmapped as a side effect of being destroyed, as destroying clears the swipe tracker reference. --- js/ui/appDisplay.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/js/ui/appDisplay.js b/js/ui/appDisplay.js index 0c2b975987..e6571555fd 100644 --- a/js/ui/appDisplay.js +++ b/js/ui/appDisplay.js @@ -1023,7 +1023,8 @@ var BaseAppView = GObject.registerClass({ } vfunc_unmap() { - this._swipeTracker.enabled = false; + if (this._swipeTracker) + this._swipeTracker.enabled = false; this._clearAnimateLater(); this._disconnectDnD(); super.vfunc_unmap(); -- GitLab From e89de3d793839b158ffe8df8bae21bd199bc94f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 12:04:24 +0100 Subject: [PATCH 17/25] overviewControls: Handle shutdown gracefully There are two adaptations needed here: * Handle the WorkspaceDisplay being destroyed This happens as a side effect of being a child actor. Handle this by removing the reference to it, and handling being unmapped more gracefully by not failing to hide the workspace display if it was already destroyed. * Handle setting the container to 'null' This is part of being destroyed, so handle it. --- js/ui/overviewControls.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/ui/overviewControls.js b/js/ui/overviewControls.js index 72542b3813..05349a5558 100644 --- a/js/ui/overviewControls.js +++ b/js/ui/overviewControls.js @@ -112,7 +112,8 @@ class ControlsManagerLayout extends Clutter.BoxLayout { vfunc_set_container(container) { this._container = container; - this.hookup_style(container); + if (container) + this.hookup_style(container); } vfunc_allocate(container, box) { @@ -603,6 +604,7 @@ class ControlsManager extends St.Widget { } _onDestroy() { + delete this._workspacesDisplay; global.workspace_manager.disconnect(this._nWorkspacesNotifyId); } @@ -620,7 +622,7 @@ class ControlsManager extends St.Widget { } vfunc_unmap() { - this._workspacesDisplay.hide(); + this._workspacesDisplay?.hide(); super.vfunc_unmap(); } -- GitLab From 5ba3fb43fd780a1b8f79a7dc9a9e7c0786a8cf9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 09:02:46 +0100 Subject: [PATCH 18/25] calendar: Clean up state on shutdown This includes destroying things before the GC takes over, and unbinding GSettings instead of destroying it, avoiding confused internal GSignal state. --- js/ui/calendar.js | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/js/ui/calendar.js b/js/ui/calendar.js index 710efbac45..bb3ef1537a 100644 --- a/js/ui/calendar.js +++ b/js/ui/calendar.js @@ -912,8 +912,7 @@ class DoNotDisturbSwitch extends PopupMenu.Switch { Gio.SettingsBindFlags.INVERT_BOOLEAN); this.connect('destroy', () => { - this._settings.run_dispose(); - this._settings = null; + Gio.Settings.unbind(this._settings, 'show-banners'); }); } }); @@ -996,6 +995,15 @@ class CalendarMessageList extends St.Widget { this._addSection(this._notificationSection); Main.sessionMode.connect('updated', this._sync.bind(this)); + global.connect('closing', this._onClosing.bind(this)); + } + + _onClosing() { + for (const section of this._sectionList) { + for (const id of section._calendarConnectionsIds) + section.disconnect(id); + } + this._sectionList.remove_all_children(); } _addSection(section) { @@ -1014,6 +1022,7 @@ class CalendarMessageList extends St.Widget { this._sectionList.remove_actor(section); })); + section._calendarConnectionsIds = connectionsIds; this._sectionList.add_actor(section); } -- GitLab From 20d1135f1b30265fc4fb42ca86d8540e43461a06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 11:24:35 +0100 Subject: [PATCH 19/25] popupMenu: Destroy both the switch and label on destruction Otherwise one will be dangling, as it's detached, meaning only the GC will deal with it. This causes issues if that happens after the mutter backend is destroyed, as that means e.g. the ClutterText of the StLabel can't disconnect its signals from the clutter backend. --- js/ui/popupMenu.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js index 11528560d4..d82b33732c 100644 --- a/js/ui/popupMenu.js +++ b/js/ui/popupMenu.js @@ -379,6 +379,12 @@ var PopupSwitchMenuItem = GObject.registerClass({ this._statusBin.child = this._switch; } + destroy() { + this._statusLabel.destroy(); + this._switch.destroy(); + super.destroy(); + } + setStatus(text) { if (text != null) { this._statusLabel.text = text; -- GitLab From fc93f71145a106d564ef69554c6c309feb1899d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 15:52:48 +0100 Subject: [PATCH 20/25] network: Handle graceful shutdown This means disconnecting signals, handling menu items being destroyed earlier than expected, destroying device wrappers. Overall, all for avoiding trying to handle state changes during tear down better. --- js/ui/status/network.js | 87 ++++++++++++++++++++++++++++++++--------- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/js/ui/status/network.js b/js/ui/status/network.js index bb119f690c..4cbf99b58c 100644 --- a/js/ui/status/network.js +++ b/js/ui/status/network.js @@ -119,17 +119,27 @@ var NMConnectionItem = class { this._sync(); } - _buildUI() { - this.labelItem = new PopupMenu.PopupMenuItem(''); + _setLabelItem(item) { + this.labelItem = item; this.labelItem.connect('activate', this._toggle.bind(this)); + this.labelItem.connect('destroy', () => delete this.labelItem); + } - this.radioItem = new PopupMenu.PopupMenuItem(this._connection.get_id(), false); + _setRadioItem(item) { + this.radioItem = item; this.radioItem.connect('activate', this._activate.bind(this)); + this.radioItem.connect('destroy', () => delete this.radioItem); + } + + _buildUI() { + this._setLabelItem(new PopupMenu.PopupMenuItem('')); + this._setRadioItem(new PopupMenu.PopupMenuItem(this._connection.get_id(), false)); } destroy() { - this.labelItem.destroy(); - this.radioItem.destroy(); + this.labelItem?.destroy(); + this.radioItem?.destroy(); + this._clearActiveConnection(); } updateForConnection(connection) { @@ -183,11 +193,16 @@ var NMConnectionItem = class { this._sync(); } - setActiveConnection(activeConnection) { + _clearActiveConnection() { if (this._activeConnectionChangedId > 0) { this._activeConnection.disconnect(this._activeConnectionChangedId); this._activeConnectionChangedId = 0; + this._activeConnection = null; } + } + + setActiveConnection(activeConnection) { + this._clearActiveConnection(); this._activeConnection = activeConnection; @@ -217,6 +232,7 @@ var NMConnectionSection = class NMConnectionSection { this.item = new PopupMenu.PopupSubMenuMenuItem('', true); this.item.menu.addMenuItem(this._labelSection); this.item.menu.addMenuItem(this._radioSection); + this.item.connect('destroy', () => delete this.item); this._notifyConnectivityId = this._client.connect('notify::connectivity', this._iconChanged.bind(this)); } @@ -227,7 +243,7 @@ var NMConnectionSection = class NMConnectionSection { this._notifyConnectivityId = 0; } - this.item.destroy(); + this.item?.destroy(); } _iconChanged() { @@ -361,6 +377,8 @@ var NMConnectionDevice = class NMConnectionDevice extends NMConnectionSection { } destroy() { + this.emit('destroy'); + if (this._stateChangedId) { GObject.signal_handler_disconnect(this._device, this._stateChangedId); this._stateChangedId = 0; @@ -490,6 +508,7 @@ var NMConnectionDevice = class NMConnectionDevice extends NMConnectionSection { } } }; +Signals.addSignalMethods(NMConnectionDevice.prototype); var NMDeviceWired = class extends NMConnectionDevice { constructor(client, device) { @@ -1204,6 +1223,7 @@ var NMDeviceWireless = class { this.item = new PopupMenu.PopupSubMenuMenuItem('', true); this.item.menu.addAction(_("Select Network"), this._showDialog.bind(this)); + this.item.connect('destroy', () => delete this.item); this._toggleItem = new PopupMenu.PopupMenuItem(''); this._toggleItem.connect('activate', this._toggleWifi.bind(this)); @@ -1230,6 +1250,8 @@ var NMDeviceWireless = class { } destroy() { + this.emit('destroy'); + if (this._activeApChangedId) { GObject.signal_handler_disconnect(this._device, this._activeApChangedId); this._activeApChangedId = 0; @@ -1259,7 +1281,7 @@ var NMDeviceWireless = class { this._notifyConnectivityId = 0; } - this.item.destroy(); + this.item?.destroy(); } _deviceStateChanged(device, newstate, oldstate, reason) { @@ -1415,12 +1437,14 @@ var NMVpnConnectionItem = class extends NMConnectionItem { return this._activeConnection.vpn_state != NM.VpnConnectionState.DISCONNECTED; } - _buildUI() { - this.labelItem = new PopupMenu.PopupMenuItem(''); - this.labelItem.connect('activate', this._toggle.bind(this)); + destroy() { + this._clearActiveConnection(); + super.destroy(); + } - this.radioItem = new PopupMenu.PopupSwitchMenuItem(this._connection.get_id(), false); - this.radioItem.connect('toggled', this._toggle.bind(this)); + _buildUI() { + this._setLabelItem(new PopupMenu.PopupMenuItem('')); + this._setRadioItem(new PopupMenu.PopupSwitchMenuItem(this._connection.get_id(), false)); } _sync() { @@ -1466,11 +1490,15 @@ var NMVpnConnectionItem = class extends NMConnectionItem { super._connectionStateChanged(); } - setActiveConnection(activeConnection) { + _clearActiveConnection() { if (this._activeConnectionChangedId > 0) { this._activeConnection.disconnect(this._activeConnectionChangedId); this._activeConnectionChangedId = 0; } + } + + setActiveConnection(activeConnection) { + this._clearActiveConnection(); this._activeConnection = activeConnection; @@ -1503,6 +1531,12 @@ var NMVpnSection = class extends NMConnectionSection { this._sync(); } + destroy() { + for (const item of this._connectionItems.values()) + item.destroy(); + super.destroy(); + } + _sync() { let nItems = this._connectionItems.size; this.item.visible = nItems > 0; @@ -1655,6 +1689,18 @@ class Indicator extends PanelMenu.SystemIndicator { this._ctypes[NM.SETTING_VPN_SETTING_NAME] = NMConnectionCategory.VPN; this._getClient(); + this.connect('destroy', this._onDestroy.bind(this)); + } + + _onDestroy() { + for (const device of this._nmDevices) + device._delegate?.destroy(); + this._vpnSection.destroy(); + + if (this._mainConnectionStateChangedId > 0) { + this._mainConnection.disconnect(this._mainConnectionStateChangedId); + this._mainConnectionStateChangedId = 0; + } } async _getClient() { @@ -1780,15 +1826,20 @@ class Indicator extends PanelMenu.SystemIndicator { let wrapperClass = this._dtypes[device.get_device_type()]; if (wrapperClass) { let wrapper = new wrapperClass(this._client, device); + const notifyInterfaceId = + device.connect('notify::interface', () => { + this._deviceChanged(device, false); + }); + wrapper.connect('destroy', + () => { + GObject.signal_handler_disconnect(device, notifyInterfaceId); + }); + device._delegate = wrapper; this._addDeviceWrapper(wrapper); this._nmDevices.push(device); this._deviceChanged(device, skipSyncDeviceNames); - - device.connect('notify::interface', () => { - this._deviceChanged(device, false); - }); } } -- GitLab From 4d651ed5ba536bb99f062c39d1b2f3f6e4702c03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 09:17:15 +0100 Subject: [PATCH 21/25] messageList: Remove messages on shutdown This avoids a bunch of warnings that would otherwise happen during the last GC sweep. --- js/ui/messageList.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/js/ui/messageList.js b/js/ui/messageList.js index 8ffdaebc13..921033f80f 100644 --- a/js/ui/messageList.js +++ b/js/ui/messageList.js @@ -566,6 +566,7 @@ var MessageListSection = GObject.registerClass({ let id = Main.sessionMode.connect('updated', this._sync.bind(this)); + global.connect('closing', this._onClosing.bind(this)); this.connect('destroy', () => { Main.sessionMode.disconnect(id); }); @@ -575,6 +576,11 @@ var MessageListSection = GObject.registerClass({ this._sync(); } + _onClosing() { + for (const message of this._messages) + this.removeMessage(message, false); + } + get empty() { return this._empty; } -- GitLab From 1dec9d2809aad7bfb4ab983f40302cae44b87835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 14:31:02 +0100 Subject: [PATCH 22/25] panel: Make sure to destroy indicators when destroying panel Otherwise we end up trying to handle things when cleaned up from the GC. --- js/ui/panel.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/js/ui/panel.js b/js/ui/panel.js index cba32416a2..af3a3a3456 100644 --- a/js/ui/panel.js +++ b/js/ui/panel.js @@ -775,6 +775,13 @@ class Panel extends St.Widget { global.display.connect('workareas-changed', () => this.queue_relayout()); this._updatePanel(); + + this.connect('destroy', this._onDestroy.bind(this)); + } + + _onDestroy() { + for (const role in PANEL_ITEM_IMPLEMENTATIONS) + this.statusArea[role].destroy(); } vfunc_get_preferred_width(_forHeight) { @@ -1009,6 +1016,9 @@ class Panel extends St.Widget { return null; } indicator = new constructor(this); + indicator.connect('destroy', () => { + indicator.menu?.disconnect(indicator.menu._openChangedId); + }); this.statusArea[role] = indicator; } return indicator; -- GitLab From 4a360f20d8a5e2f3803ac8b4a9b2f1f7cdcb851e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 14:31:39 +0100 Subject: [PATCH 23/25] panelMenu: Disconnect signals when menu is destroying Otherwise we end up trying to handle menu changes during GC. --- js/ui/panelMenu.js | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/js/ui/panelMenu.js b/js/ui/panelMenu.js index 0c2478f8db..0d46cd95aa 100644 --- a/js/ui/panelMenu.js +++ b/js/ui/panelMenu.js @@ -116,15 +116,30 @@ var Button = GObject.registerClass({ this.track_hover = sensitive; } + _clearMenu() { + if (!this.menu) + return; + + if (this._openStateChangedId) + this.menu.disconnect(this._openStateChangedId); + if (this._keyPressEventId) + this.menu.actor.disconnect(this._keyPressEventId); + this._openStateChangedId = undefined; + this._keyPressEventId = undefined; + this.menu.destroy(); + this.menu = null; + } + setMenu(menu) { - if (this.menu) - this.menu.destroy(); + this._clearMenu(); this.menu = menu; if (this.menu) { this.menu.actor.add_style_class_name('panel-menu'); - this.menu.connect('open-state-changed', this._onOpenStateChanged.bind(this)); - this.menu.actor.connect('key-press-event', this._onMenuKeyPress.bind(this)); + this._openStateChangedId = + this.menu.connect('open-state-changed', this._onOpenStateChanged.bind(this)); + this._keyPressEventId = + this.menu.actor.connect('key-press-event', this._onMenuKeyPress.bind(this)); Main.uiGroup.add_actor(this.menu.actor); this.menu.actor.hide(); @@ -185,8 +200,7 @@ var Button = GObject.registerClass({ } _onDestroy() { - if (this.menu) - this.menu.destroy(); + this._clearMenu(); super._onDestroy(); } }); -- GitLab From de8810500d9a5184b5c00462ba20e547a6d6a05a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Fri, 12 Mar 2021 14:33:17 +0100 Subject: [PATCH 24/25] searchController: Disconnect notify::key-focus handler when destroyed Otherwise during destruction we might end up trying to handle key focus changes after the search controller was teared down. --- js/ui/searchController.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/js/ui/searchController.js b/js/ui/searchController.js index 5d25897c79..80f7d7c711 100644 --- a/js/ui/searchController.js +++ b/js/ui/searchController.js @@ -68,7 +68,9 @@ var SearchController = GObject.registerClass({ this._searchResults.popupMenuDefault(); }); this._entry.connect('notify::mapped', this._onMapped.bind(this)); - global.stage.connect('notify::key-focus', this._onStageKeyFocusChanged.bind(this)); + const keyFocusId = + global.stage.connect('notify::key-focus', this._onStageKeyFocusChanged.bind(this)); + this.connect('destroy', () => global.stage.disconnect(keyFocusId)); this._entry.set_primary_icon(new St.Icon({ style_class: 'search-entry-icon', -- GitLab From 7449295365992a80c6028a2e984eeb029aeb7cb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Sat, 13 Mar 2021 12:24:05 +0100 Subject: [PATCH 25/25] popupMenu: Handle components already being gone when destroying This might happen on shutdown, when all actors owned by the shell are destroyed from from the uiGroup. --- js/ui/popupMenu.js | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/js/ui/popupMenu.js b/js/ui/popupMenu.js index d82b33732c..62dc1574e1 100644 --- a/js/ui/popupMenu.js +++ b/js/ui/popupMenu.js @@ -471,6 +471,8 @@ var PopupMenuBase = class { throw new TypeError('Cannot instantiate abstract class %s'.format(this.constructor.name)); this.sourceActor = sourceActor; + if (sourceActor) + this.sourceActor.connect('destroy', () => delete this.sourceActor); this.focusActor = sourceActor; this._parent = null; @@ -479,6 +481,7 @@ var PopupMenuBase = class { x_expand: true, y_expand: true, }); + this.box.connect('destroy', () => delete this.box); if (styleClass !== undefined) this.box.style_class = styleClass; @@ -779,6 +782,9 @@ var PopupMenuBase = class { } _getMenuItems() { + if (!this.box) + return []; + return this.box.get_children().map(a => a._delegate).filter(item => { return item instanceof PopupBaseMenuItem || item instanceof PopupMenuSection; }); @@ -814,7 +820,7 @@ var PopupMenuBase = class { destroy() { this.close(); this.removeAll(); - this.actor.destroy(); + this.actor?.destroy(); this.emit('destroy'); @@ -832,6 +838,10 @@ var PopupMenu = class extends PopupMenuBase { this._arrowSide = arrowSide; this._boxPointer = new BoxPointer.BoxPointer(arrowSide); + this._boxPointer.connect('destroy', () => { + this._boxPointer = null; + this.actor = null; + }); this.actor = this._boxPointer; this.actor._delegate = this; this.actor.style_class = 'popup-menu-boxpointer'; @@ -850,6 +860,11 @@ var PopupMenu = class extends PopupMenuBase { if (!this.sourceActor.mapped) this.close(); }); + this.sourceActor.connect('destroy', + () => { + this._keyPressId = 0; + this._notifyMappedId = 0; + }); } this._systemModalOpenedId = 0; @@ -947,7 +962,7 @@ var PopupMenu = class extends PopupMenuBase { if (this._activeMenuItem) this._activeMenuItem.active = false; - if (this._boxPointer.visible) { + if (this._boxPointer?.visible) { this._boxPointer.close(animate, () => { this.emit('menu-closed'); }); @@ -1156,6 +1171,7 @@ var PopupMenuSection = class extends PopupMenuBase { super(); this.actor = this.box; + this.actor.connect('destroy', () => delete this.actor); this.actor._delegate = this; this.isOpen = true; } @@ -1342,9 +1358,9 @@ var PopupMenuManager = class { menu.disconnect(menudata.destroyId); if (menudata.enterId) - menu.sourceActor.disconnect(menudata.enterId); + menu.sourceActor?.disconnect(menudata.enterId); if (menudata.focusInId) - menu.sourceActor.disconnect(menudata.focusInId); + menu.sourceActor?.disconnect(menudata.focusInId); if (menu.sourceActor) this._grabHelper.removeActor(menu.sourceActor); -- GitLab