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
This commit is contained in:
Jonas Ådahl 2017-09-06 11:10:27 +08:00
parent 656f64f3a5
commit 634f48a1cf
2 changed files with 93 additions and 11 deletions

View File

@ -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;

View File

@ -76,6 +76,9 @@ static gboolean
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