[mutter/gnome-3-26] monitor-manager: Don't free old state until logical monitors are replaced

[Jonas Ådahl <jadahl src gnome org>]

commit 634f48a1cf20ae8f4ccc5eef2c52f7566bad0382
Author: Jonas Ådahl <jadahl gmail com>
Date:   Wed Sep 6 11:10:27 2017 +0800

    monitor-manager: Don't free old state until logical monitors are replaced
    
    Logical monitors keep pointers around to monitor objects, which themself
    keep pointers aronud to outputs, which keeps pointer to modes and CRTCs.
    To avoid causing crashes when using the logical monitor API (which
    might use monitor APIs etc) between a hot plug and the time the logical
    monitors are regenerated (which is done synchronously in the native
    backend but asynchronously in the X11 backend), postpone the freeing of
    the state that logical monitor references, until the logical monitor
    state is regenerated.
    
    On the native backend, this should have no significant effect, as the
    logical state is always regenerated immediately after the hardware
    state is updated, but on X11, this should fix race conditions when
    events being processed between the hot plug and the hot plug result
    tries to access the yet to be up to date state.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=786929

 src/backends/meta-monitor-manager-private.h |   13 ++++
 src/backends/meta-monitor-manager.c         |   91 +++++++++++++++++++++++---
 2 files changed, 93 insertions(+), 11 deletions(-)
---
diff --git a/src/backends/meta-monitor-manager-private.h b/src/backends/meta-monitor-manager-private.h
index 072e765..6f6d7b7 100644
--- a/src/backends/meta-monitor-manager-private.h
+++ b/src/backends/meta-monitor-manager-private.h
@@ -328,6 +328,19 @@ struct _MetaMonitorManager
 
   GList *monitors;
 
+  struct {
+    MetaOutput *outputs;
+    unsigned int n_outputs;
+
+    MetaCrtcMode *modes;
+    unsigned int n_modes;
+
+    MetaCrtc *crtcs;
+    unsigned int n_crtcs;
+
+    GList *monitors;
+  } pending_cleanup;
+
   GList *logical_monitors;
   MetaLogicalMonitor *primary_logical_monitor;
 
diff --git a/src/backends/meta-monitor-manager.c b/src/backends/meta-monitor-manager.c
index df51ca0..f2ad3f3 100644
--- a/src/backends/meta-monitor-manager.c
+++ b/src/backends/meta-monitor-manager.c
@@ -77,6 +77,9 @@ meta_monitor_manager_is_config_complete (MetaMonitorManager *manager,
                                          MetaMonitorsConfig *config);
 
 static void
+cleanup_pending_cleanup_state (MetaMonitorManager *manager);
+
+static void
 meta_monitor_manager_init (MetaMonitorManager *manager)
 {
 }
@@ -802,6 +805,8 @@ meta_monitor_manager_finalize (GObject *object)
   meta_monitor_manager_free_crtc_array (manager->crtcs, manager->n_crtcs);
   g_list_free_full (manager->logical_monitors, g_object_unref);
 
+  cleanup_pending_cleanup_state (manager);
+
   g_signal_handler_disconnect (meta_get_backend (),
                                manager->experimental_features_changed_handler_id);
 
@@ -2482,16 +2487,10 @@ meta_monitor_manager_get_screen_size (MetaMonitorManager *manager,
 }
 
 static void
-rebuild_monitors (MetaMonitorManager *manager)
+generate_monitors (MetaMonitorManager *manager)
 {
   unsigned int i;
 
-  if (manager->monitors)
-    {
-      g_list_free_full (manager->monitors, g_object_unref);
-      manager->monitors = NULL;
-    }
-
   for (i = 0; i < manager->n_outputs; i++)
     {
       MetaOutput *output = &manager->outputs[i];
@@ -2551,6 +2550,37 @@ meta_monitor_manager_is_transform_handled (MetaMonitorManager  *manager,
   return manager_class->is_transform_handled (manager, crtc, transform);
 }
 
+static void
+cleanup_pending_cleanup_state (MetaMonitorManager *manager)
+{
+  if (manager->pending_cleanup.monitors)
+    {
+      g_list_free_full (manager->pending_cleanup.monitors, g_object_unref);
+      manager->pending_cleanup.monitors = NULL;
+    }
+  if (manager->pending_cleanup.outputs)
+    {
+      meta_monitor_manager_free_output_array (manager->pending_cleanup.outputs,
+                                              manager->pending_cleanup.n_outputs);
+      manager->pending_cleanup.outputs = NULL;
+      manager->pending_cleanup.n_outputs = 0;
+    }
+  if (manager->pending_cleanup.modes)
+    {
+      meta_monitor_manager_free_mode_array (manager->pending_cleanup.modes,
+                                            manager->pending_cleanup.n_modes);
+      manager->pending_cleanup.modes = NULL;
+      manager->pending_cleanup.n_modes = 0;
+    }
+  if (manager->pending_cleanup.crtcs)
+    {
+      meta_monitor_manager_free_crtc_array (manager->pending_cleanup.crtcs,
+                                            manager->pending_cleanup.n_crtcs);
+      manager->pending_cleanup.crtcs = NULL;
+      manager->pending_cleanup.n_crtcs = 0;
+    }
+}
+
 void
 meta_monitor_manager_read_current_state (MetaMonitorManager *manager)
 {
@@ -2572,11 +2602,46 @@ meta_monitor_manager_read_current_state (MetaMonitorManager *manager)
   manager->serial++;
   META_MONITOR_MANAGER_GET_CLASS (manager)->read_current (manager);
 
-  rebuild_monitors (manager);
+  /*
+   * We must delay cleaning up the old hardware state, because the current
+   * logical state, when running on top of X11/Xrandr, may still be based on it
+   * for some time. The logical state may not be updated immediately, in case
+   * it is reconfigured, but after getting the response from a logical state
+   * configuration request to the X server. To be able to handle events and
+   * other things needing the logical state between the request and the
+   * response, the hardware state the logical state points to must be kept
+   * alive.
+   *
+   * If there is already a hardware state pending cleaning up, it means that
+   * the current logical state is still using that hardware state, so we can
+   * destroy the just replaced state immedietaley.
+   */
+  if (manager->pending_cleanup.outputs ||
+      manager->pending_cleanup.crtcs ||
+      manager->pending_cleanup.monitors)
+    {
+      if (manager->monitors)
+        {
+          g_list_free_full (manager->monitors, g_object_unref);
+          manager->monitors = NULL;
+        }
+      meta_monitor_manager_free_output_array (old_outputs, n_old_outputs);
+      meta_monitor_manager_free_mode_array (old_modes, n_old_modes);
+      meta_monitor_manager_free_crtc_array (old_crtcs, n_old_crtcs);
+    }
+  else
+    {
+      manager->pending_cleanup.outputs = old_outputs;
+      manager->pending_cleanup.n_outputs = n_old_outputs;
+      manager->pending_cleanup.modes = old_modes;
+      manager->pending_cleanup.n_modes = n_old_modes;
+      manager->pending_cleanup.crtcs = old_crtcs;
+      manager->pending_cleanup.n_crtcs = n_old_crtcs;
+      manager->pending_cleanup.monitors = manager->monitors;
+      manager->monitors = NULL;
+    }
 
-  meta_monitor_manager_free_output_array (old_outputs, n_old_outputs);
-  meta_monitor_manager_free_mode_array (old_modes, n_old_modes);
-  meta_monitor_manager_free_crtc_array (old_crtcs, n_old_crtcs);
+  generate_monitors (manager);
 }
 
 static void
@@ -2668,6 +2733,8 @@ meta_monitor_manager_rebuild (MetaMonitorManager *manager,
   meta_monitor_manager_notify_monitors_changed (manager);
 
   g_list_free_full (old_logical_monitors, g_object_unref);
+
+  cleanup_pending_cleanup_state (manager);
 }
 
 static void
@@ -2710,6 +2777,8 @@ meta_monitor_manager_rebuild_derived (MetaMonitorManager *manager,
   meta_monitor_manager_notify_monitors_changed (manager);
 
   g_list_free_full (old_logical_monitors, g_object_unref);
+
+  cleanup_pending_cleanup_state (manager);
 }
 
 void