cms-colord used the weston_compositor destroy signal to trigger its final colord_module_destroy cleanup, and the wl_output destroy signal to trigger per output cleanup.
The problem is that the compositor destroy signal gets emitted before the output destroy signals at compositor shutdown, colord_module_destroy would free all its shared data structures and then later on the output destroy callback would try to access those shared data structures when handling output destruction -> Use after free -> Crash, usually with VT switching dead and thereby an unuseable system requiring a reboot. Solve this by moving the output destruction handling into the colord_cms_output_destroy() cleanup function for colord-cms own hash dictionary of all active outputs. The output destroy callback just removes the corresponding output from the dictionary and triggers proper cleanup if an output is unplugged during runtime. During compositor shutdown, the dictionary as a whole is released before releasing all other shared data structures, thereby triggering cleanup of all remaining outputs. Tested to fix crashes on x11 and drm backends. Signed-off-by: Mario Kleiner <[email protected]> Cc: Richard Hughes <[email protected]> --- src/cms-colord.c | 53 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/src/cms-colord.c b/src/cms-colord.c index 2adc886..b0dbb7b 100644 --- a/src/cms-colord.c +++ b/src/cms-colord.c @@ -220,26 +220,9 @@ colord_notifier_output_destroy(struct wl_listener *listener, void *data) container_of(listener, struct cms_output, destroy_listener); struct weston_output *o = (struct weston_output *) data; struct cms_colord *cms = ocms->cms; - - gboolean ret; gchar *device_id; - GError *error = NULL; - colord_idle_cancel_for_output(cms, o); device_id = get_output_id(cms, o); - weston_log("colord: output removed %s\n", device_id); - g_signal_handlers_disconnect_by_data(ocms->device, ocms); - ret = cd_client_delete_device_sync (cms->client, - ocms->device, - NULL, - &error); - - if (!ret) { - weston_log("colord: failed to delete device: %s\n", error->message); - g_error_free(error); - goto out; - } -out: g_hash_table_remove (cms->devices, device_id); g_free (device_id); } @@ -445,15 +428,17 @@ colord_load_pnp_ids(struct cms_colord *cms) static void colord_module_destroy(struct cms_colord *cms) { - g_free(cms->pnp_ids_data); - g_hash_table_unref(cms->pnp_ids); - if (cms->loop) { g_main_loop_quit(cms->loop); g_main_loop_unref(cms->loop); } if (cms->thread) g_thread_join(cms->thread); + + /* cms->devices must be destroyed before other resources, as + * the other resources are needed during output cleanup in + * cms->devices unref. + */ if (cms->devices) g_hash_table_unref(cms->devices); if (cms->client) @@ -462,6 +447,10 @@ colord_module_destroy(struct cms_colord *cms) close(cms->readfd); if (cms->writefd) close(cms->writefd); + + g_free(cms->pnp_ids_data); + g_hash_table_unref(cms->pnp_ids); + free(cms); } @@ -477,8 +466,32 @@ static void colord_cms_output_destroy(gpointer data) { struct cms_output *ocms = (struct cms_output *) data; + struct cms_colord *cms = ocms->cms; + struct weston_output *o = ocms->o; + gboolean ret; + gchar *device_id; + GError *error = NULL; + + colord_idle_cancel_for_output(cms, o); + device_id = get_output_id(cms, o); + weston_log("colord: output unplugged %s\n", device_id); + + wl_list_remove(&ocms->destroy_listener.link); + g_signal_handlers_disconnect_by_data(ocms->device, ocms); + + ret = cd_client_delete_device_sync (cms->client, + ocms->device, + NULL, + &error); + + if (!ret) { + weston_log("colord: failed to delete device: %s\n", error->message); + g_error_free(error); + } + g_object_unref(ocms->device); g_slice_free(struct cms_output, ocms); + g_free (device_id); } WL_EXPORT int -- 1.9.1 _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
