Skip to content
  • Pekka Paalanen's avatar
    text_backend: make destructor call explicit · aa9536a9
    Pekka Paalanen authored
    
    
    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: default avatarPekka Paalanen <pekka.paalanen@collabora.co.uk>
    Reviewed-By: default avatarDerek Foreman <derekf@osg.samsung.com>
    aa9536a9