From a6c2ba3d3cb53d176af4721b32e32a338dbd1ca1 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 1/8] 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 efe1271b5a..770794842a 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -1290,7 +1290,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 ab4df88fedb702595222a6a358cabb334fae99c6 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 2/8] 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 | 60 ---------------------------------------------- src/shell-global.c | 35 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 60 deletions(-) diff --git a/src/main.c b/src/main.c index 03708fc8fe..4d97603fbd 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) @@ -552,8 +496,6 @@ main (int argc, char **argv) dump_gjs_stack_on_signal (SIGSEGV); } - shell_profiler_init (); - if (meta_context_get_compositor_type (context) == META_COMPOSITOR_TYPE_WAYLAND) meta_context_raise_rlimit_nofile (context, NULL); @@ -571,8 +513,6 @@ main (int argc, char **argv) meta_context_destroy (g_steal_pointer (&context)); - 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 770794842a..b6f6610d7f 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -353,6 +353,39 @@ switcheroo_vanished_cb (GDBusConnection *connection, g_object_notify_by_pspec (G_OBJECT (global), props[PROP_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) { @@ -455,6 +488,8 @@ shell_global_init (ShellGlobal *global) switcheroo_vanished_cb, global, NULL); + + shell_global_profiler_init (global); } static void -- GitLab From 4a8b1157df7dbb05138d58b807c39f77a5bd9e8b 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 3/8] 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 | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/shell-global.c b/src/shell-global.c index b6f6610d7f..209b0ba066 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -493,26 +493,34 @@ shell_global_init (ShellGlobal *global) } static void -shell_global_finalize (GObject *object) +shell_global_dispose (GObject *object) { ShellGlobal *global = SHELL_GLOBAL (object); g_clear_object (&global->js_context); - g_object_unref (global->settings); - - the_object = NULL; 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); } @@ -524,6 +532,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 9ff73c7952b9727d3814189835c22420f4f25532 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 4/8] 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. We need to act like this because at this point the global object will have two references, one is the original instance we constructed and the other one is owned by the JS context itself. This leads to a reference cycle, and so we've to break it in this way. 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 4d97603fbd..a32e48913a 100644 --- a/src/main.c +++ b/src/main.c @@ -514,8 +514,7 @@ main (int argc, char **argv) meta_context_destroy (g_steal_pointer (&context)); 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 209b0ba066..8d4a4d4ff2 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -733,17 +733,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_object (&self->js_context); + g_object_run_dispose (G_OBJECT (global)); + g_object_unref (global); } static guint32 -- GitLab From fee1bc82da899550b02ab67aae438a5487014b15 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 5/8] 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 a32e48913a..9001ce6651 100644 --- a/src/main.c +++ b/src/main.c @@ -342,7 +342,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 57d0e46f93dac404bc67530bc6a3df49ea8ac046 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 6/8] 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 6920ff8ceeec491f41f3ff0f0461689f46f77953 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 7/8] 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 9d47aec2365d80f5472c6ba3cf3babc9dbaf9117 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 8/8] global: Add Shell global setter with nesting support _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 it a setter for the global object that returns the passed object. Change this in both main and run-js-test. --- src/main.c | 10 ++++++---- src/run-js-test.c | 3 +-- src/shell-global-private.h | 4 ++-- src/shell-global.c | 30 ++++++++++-------------------- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/main.c b/src/main.c index 9001ce6651..aaf88fd3a6 100644 --- a/src/main.c +++ b/src/main.c @@ -331,8 +331,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_set (g_object_new (SHELL_TYPE_GLOBAL, NULL)); context = _shell_global_get_gjs_context (global); shell_introspection_init (); @@ -433,6 +432,7 @@ main (int argc, char **argv) { g_autoptr (MetaContext) context = NULL; GError *error = NULL; + ShellGlobal *global; int ecode = EXIT_SUCCESS; bindtextdomain (GETTEXT_PACKAGE, LOCALEDIR); @@ -484,7 +484,9 @@ 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_set (g_object_new (SHELL_TYPE_GLOBAL, + "session-mode", session_mode, + NULL)); dump_gjs_stack_on_signal (SIGABRT); dump_gjs_stack_on_signal (SIGFPE); @@ -515,7 +517,7 @@ main (int argc, char **argv) meta_context_destroy (g_steal_pointer (&context)); 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..86157ff766 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_set (g_object_new (SHELL_TYPE_GLOBAL, 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..e213348966 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_set (ShellGlobal *global); + void _shell_global_set_plugin (ShellGlobal *global, MetaPlugin *plugin); diff --git a/src/shell-global.c b/src/shell-global.c index 8d4a4d4ff2..66d5e412fa 100644 --- a/src/shell-global.c +++ b/src/shell-global.c @@ -689,34 +689,24 @@ shell_global_class_init (ShellGlobalClass *klass) } /* - * _shell_global_init: (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 + * _shell_global_set: (skip) + * @global: the object to be used as global singleton * - * Initializes the shell global singleton with the construction-time - * properties. - * - * There are currently no such properties, so @first_property_name should - * always be %NULL. + * Set the shell global singleton. * * 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_set (ShellGlobal *global) { - 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, - first_property_name, - argument_list)); - va_end (argument_list); + the_object = global; + return the_object; } /** -- GitLab