text_backend: make destructor call explicit

We used to rely on the order in which the
weston_compositor::destroy_signal callbacks happened, to not access
freed memory. Don't know when, but this broke at least with ivi-shell,
which caused crashes in random places on compositor shutdown.

Valgrind found the following:

 Invalid write of size 8
    at 0xC2EDC69: unbind_input_panel (input-panel-ivi.c:340)
    by 0x4E3B6BB: destroy_resource (wayland-server.c:537)
    by 0x4E3E085: for_each_helper.isra.0 (wayland-util.c:359)
    by 0x4E3E60D: wl_map_for_each (wayland-util.c:365)
    by 0x4E3BEC7: wl_client_destroy (wayland-server.c:675)
    by 0x4182F2: text_backend_notifier_destroy (text-backend.c:1047)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)
  Address 0x67ea360 is 208 bytes inside a block of size 232 free'd
    at 0x4C2A6BC: free (vg_replace_malloc.c:473)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)

 Invalid write of size 8
    at 0x4E3E0D7: wl_list_remove (wayland-util.c:57)
    by 0xC2EDEE9: destroy_input_panel_surface (input-panel-ivi.c:191)
    by 0x4E3B6BB: destroy_resource (wayland-server.c:537)
    by 0x4E3BC7B: wl_resource_destroy (wayland-server.c:550)
    by 0x40DB8B: wl_signal_emit (wayland-server-core.h:264)
    by 0x40DB8B: weston_surface_destroy (compositor.c:1883)
    by 0x40DB8B: weston_surface_destroy (compositor.c:1873)
    by 0x4E3B6BB: destroy_resource (wayland-server.c:537)
    by 0x4E3E085: for_each_helper.isra.0 (wayland-util.c:359)
    by 0x4E3E60D: wl_map_for_each (wayland-util.c:365)
    by 0x4E3BEC7: wl_client_destroy (wayland-server.c:675)
    by 0x4182F2: text_backend_notifier_destroy (text-backend.c:1047)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)
  Address 0x67ea370 is 224 bytes inside a block of size 232 free'd
    at 0x4C2A6BC: free (vg_replace_malloc.c:473)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)

 Invalid write of size 8
    at 0x4E3E0E7: wl_list_remove (wayland-util.c:58)
    by 0xC2EDEE9: destroy_input_panel_surface (input-panel-ivi.c:191)
    by 0x4E3B6BB: destroy_resource (wayland-server.c:537)
    by 0x4E3BC7B: wl_resource_destroy (wayland-server.c:550)
    by 0x40DB8B: wl_signal_emit (wayland-server-core.h:264)
    by 0x40DB8B: weston_surface_destroy (compositor.c:1883)
    by 0x40DB8B: weston_surface_destroy (compositor.c:1873)
    by 0x4E3B6BB: destroy_resource (wayland-server.c:537)
    by 0x4E3E085: for_each_helper.isra.0 (wayland-util.c:359)
    by 0x4E3E60D: wl_map_for_each (wayland-util.c:365)
    by 0x4E3BEC7: wl_client_destroy (wayland-server.c:675)
    by 0x4182F2: text_backend_notifier_destroy (text-backend.c:1047)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)
  Address 0x67ea368 is 216 bytes inside a block of size 232 free'd
    at 0x4C2A6BC: free (vg_replace_malloc.c:473)
    by 0x4084FB: wl_signal_emit (wayland-server-core.h:264)
    by 0x4084FB: main (compositor.c:5465)

Looking at the first of these, unbind_input_panel() gets called when the
text-backend destroys its helper client which has bound to input_panel
interface. This happens after the shell's destroy_signal callback has
been called, so the shell has already been freed.

The other two errors come from
  wl_list_remove(&input_panel_surface->link);
which has gone stale when the shell was destroyed
(shell->input_panel.surfaces list).

Rather than creating even more destroy listeners and hooking them up in
spaghetti, modify text-backend to not hook up to the compositor destroy
signal. Instead, make it the text_backend_init() callers' responsibility
to also call text_backend_destroy() appropriately, before the shell goes
away.

This fixed all the above Valgrind errors, and avoid a crash with
ivi-shell when exiting Weston.

Also using desktop-shell exhibited similar Valgrind errors which are
fixed by this patch, but those didn't happen to cause any crashes AFAIK.

Signed-off-by: Pekka Paalanen <pekka.paalanen@collabora.co.uk>
Reviewed-By: Derek Foreman <derekf@osg.samsung.com>

desktop-shell/shell.c

@@ -6347,6 +6347,7 @@ shell_destroy(struct wl_listener *listener, void *data)
 	wl_list_remove(&shell->idle_listener.link);
 	wl_list_remove(&shell->wake_listener.link);
 
+	text_backend_destroy(shell->text_backend);
 	input_panel_destroy(shell);
 
 	wl_list_for_each_safe(shell_output, tmp, &shell->output_list, link) {
@@ -6513,7 +6514,8 @@ module_init(struct weston_compositor *ec,
 	if (input_panel_setup(shell) < 0)
 		return -1;
 
-	if (text_backend_init(ec) < 0)
+	shell->text_backend = text_backend_init(ec);
+	if (!shell->text_backend)
 		return -1;
 
 	shell_configuration(shell);

desktop-shell/shell.h

@@ -148,6 +148,8 @@ struct desktop_shell {
 	bool showing_input_panels;
 	bool prepare_event_sent;
 
+	struct text_backend *text_backend;
+
 	struct {
 		struct weston_surface *surface;
 		pixman_box32_t cursor_rectangle;

ivi-shell/ivi-shell.c

@@ -343,6 +343,7 @@ shell_destroy(struct wl_listener *listener, void *data)
 		container_of(listener, struct ivi_shell, destroy_listener);
 	struct ivi_shell_surface *ivisurf, *next;
 
+	text_backend_destroy(shell->text_backend);
 	input_panel_destroy(shell);
 
 	wl_list_for_each_safe(ivisurf, next, &shell->ivi_surface_list, link) {
@@ -439,7 +440,8 @@ module_init(struct weston_compositor *compositor,
 	if (input_panel_setup(shell) < 0)
 		goto out_settings;
 
-	if (text_backend_init(compositor) < 0)
+	shell->text_backend = text_backend_init(compositor);
+	if (!shell->text_backend)
 		goto out_settings;
 
 	if (wl_global_create(compositor->wl_display,

ivi-shell/ivi-shell.h

@@ -35,6 +35,8 @@ struct ivi_shell
 
 	struct wl_list ivi_surface_list; /* struct ivi_shell_surface::link */
 
+	struct text_backend *text_backend;
+
 	struct wl_listener show_input_panel_listener;
 	struct wl_listener hide_input_panel_listener;
 	struct wl_listener update_input_panel_listener;

src/compositor.h

@@ -1456,9 +1456,14 @@ weston_screenshooter_shoot(struct weston_output *output, struct weston_buffer *b
 struct clipboard *
 clipboard_create(struct weston_seat *seat);
 
-int
+struct text_backend;
+
+struct text_backend *
 text_backend_init(struct weston_compositor *ec);
 
+void
+text_backend_destroy(struct text_backend *text_backend);
+
 struct weston_process;
 typedef void (*weston_process_cleanup_func_t)(struct weston_process *process,
 					    int status);

src/text-backend.c

@@ -109,7 +109,6 @@ struct text_backend {
 	} input_method;
 
 	struct wl_listener seat_created_listener;
-	struct wl_listener destroy_listener;
 };
 
 static void
@@ -1037,21 +1036,17 @@ text_backend_configuration(struct text_backend *text_backend)
 	free(client);
 }
 
-static void
+WL_EXPORT void
-text_backend_notifier_destroy(struct wl_listener *listener, void *data)
+text_backend_destroy(struct text_backend *text_backend)
 {
-	struct text_backend *text_backend =
-		container_of(listener, struct text_backend, destroy_listener);
-
 	if (text_backend->input_method.client)
 		wl_client_destroy(text_backend->input_method.client);
 
 	free(text_backend->input_method.path);
-
 	free(text_backend);
 }
 
-WL_EXPORT int
+WL_EXPORT struct text_backend *
 text_backend_init(struct weston_compositor *ec)
 {
 	struct text_backend *text_backend;
@@ -1059,7 +1054,7 @@ text_backend_init(struct weston_compositor *ec)
 
 	text_backend = zalloc(sizeof(*text_backend));
 	if (text_backend == NULL)
-		return -1;
+		return NULL;
 
 	text_backend->compositor = ec;
 
@@ -1071,10 +1066,7 @@ text_backend_init(struct weston_compositor *ec)
 	wl_signal_add(&ec->seat_created_signal,
 		      &text_backend->seat_created_listener);
 
-	text_backend->destroy_listener.notify = text_backend_notifier_destroy;
-	wl_signal_add(&ec->destroy_signal, &text_backend->destroy_listener);
-
 	text_input_manager_create(ec);
 
-	return 0;
+	return text_backend;
 }