From d31b781efb25365d6373d6c98c10815b7feca8be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20=C3=85dahl?= Date: Tue, 7 Mar 2023 00:06:30 +0100 Subject: [PATCH] monitor-manager: Restore old config in idle callback when unconfirmed We might get told to restore the old monitor configuration by the monitor configuration prompt, in case the user pressed "revert" or equivalent. This might be in response to a button press, and those happen during frame clock dispatch. If we would restore an old configuration during dispatch, it means we would reconfigure the monitors including their stage views while dispatching, which means we'd destroy the frame clock while it's dispatching. Doing that causes problems, as the frame clock isn't expecting to be destroyed mid-function. Specifically, We'd enter clutter_frame_clock_dispatch (clutter-frame-clock.c:811) frame_clock_source_dispatch (clutter-frame-clock.c:839) g_main_dispatch (gmain.c:3454) g_main_context_dispatch (gmain.c:4172) g_main_context_iterate.constprop.0 (gmain.c:4248) g_main_loop_run (gmain.c:4448) meta_context_run_main_loop (meta-context.c:482) main (main.c:663) which would first call _clutter_process_event (clutter-main.c:920) _clutter_stage_process_queued_events (clutter-stage.c:757) handle_frame_clock_before_frame (clutter-stage-view.c:1150) which would emit e.g. a button event all the way to a button press handler, which would e.g. deny the new configuration: restore_previous_config (meta-monitor-manager.c:1931) confirm_configuration (meta-monitor-manager.c:2866) meta_monitor_manager_confirm_configuration (meta-monitor-manager.c:2880) meta_plugin_complete_display_change (meta-plugin.c:172) That would then regenerate the monitor configuration and stage view layout, which would destroy the old stage view and frame clock. meta_stage_native_rebuild_views (meta-stage-native.c:68) meta_backend_native_update_screen_size (meta-backend-native.c:457) meta_backend_sync_screen_size (meta-backend.c:266) meta_backend_monitors_changed (meta-backend.c:337) meta_monitor_manager_notify_monitors_changed (meta-monitor-manager.c:3595) meta_monitor_manager_rebuild (meta-monitor-manager.c:3683) meta_monitor_manager_native_apply_monitors_config (meta-monitor-manager-native.c:343) meta_monitor_manager_apply_monitors_config (meta-monitor-manager.c:704) After returning back to the original clutter_frame_clock_dispatch() frame, various state in the frame clock will be gone and we'd crash. Part-of: --- src/backends/meta-monitor-manager-private.h | 1 + src/backends/meta-monitor-manager.c | 44 +++++++++------------ 2 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/backends/meta-monitor-manager-private.h b/src/backends/meta-monitor-manager-private.h index 56431ba60..f9cd9ae31 100644 --- a/src/backends/meta-monitor-manager-private.h +++ b/src/backends/meta-monitor-manager-private.h @@ -154,6 +154,7 @@ struct _MetaMonitorManager MetaLogicalMonitor *primary_logical_monitor; guint dbus_name_id; + guint restore_config_id; guint persistent_timeout_id; guint panel_orientation_managed : 1; diff --git a/src/backends/meta-monitor-manager.c b/src/backends/meta-monitor-manager.c index d3a794ea6..e6df87630 100644 --- a/src/backends/meta-monitor-manager.c +++ b/src/backends/meta-monitor-manager.c @@ -1363,6 +1363,7 @@ meta_monitor_manager_dispose (GObject *object) g_clear_object (&manager->config_manager); g_clear_handle_id (&manager->persistent_timeout_id, g_source_remove); + g_clear_handle_id (&manager->restore_config_id, g_source_remove); G_OBJECT_CLASS (meta_monitor_manager_parent_class)->dispose (object); } @@ -1961,12 +1962,6 @@ save_config_timeout (gpointer user_data) return G_SOURCE_REMOVE; } -static void -cancel_persistent_confirmation (MetaMonitorManager *manager) -{ - g_clear_handle_id (&manager->persistent_timeout_id, g_source_remove); -} - static void request_persistent_confirmation (MetaMonitorManager *manager) { @@ -2822,9 +2817,11 @@ meta_monitor_manager_handle_apply_monitors_config (MetaDBusDisplayConfig *skelet return TRUE; } - if (manager->persistent_timeout_id && - method != META_MONITORS_CONFIG_METHOD_VERIFY) - cancel_persistent_confirmation (manager); + if (method != META_MONITORS_CONFIG_METHOD_VERIFY) + { + g_clear_handle_id (&manager->restore_config_id, g_source_remove); + g_clear_handle_id (&manager->persistent_timeout_id, g_source_remove); + } if (!meta_monitor_manager_apply_monitors_config (manager, config, @@ -2852,28 +2849,25 @@ meta_monitor_manager_handle_apply_monitors_config (MetaDBusDisplayConfig *skelet #undef MONITOR_CONFIGS_FORMAT #undef LOGICAL_MONITOR_CONFIG_FORMAT -static void -confirm_configuration (MetaMonitorManager *manager, - gboolean confirmed) -{ - if (confirmed) - meta_monitor_config_manager_save_current (manager->config_manager); - else - restore_previous_config (manager); -} - void meta_monitor_manager_confirm_configuration (MetaMonitorManager *manager, gboolean ok) { if (!manager->persistent_timeout_id) - { - /* too late */ - return; - } + return; - cancel_persistent_confirmation (manager); - confirm_configuration (manager, ok); + g_clear_handle_id (&manager->restore_config_id, g_source_remove); + g_clear_handle_id (&manager->persistent_timeout_id, g_source_remove); + + if (ok) + { + meta_monitor_config_manager_save_current (manager->config_manager); + } + else + { + manager->restore_config_id = + g_idle_add_once ((GSourceOnceFunc) restore_previous_config, manager); + } } static gboolean