[PATCH libinput v2 2/2] touchpad: Use remove callback to unlink event listener and stop timers

[Peter Hutterer]

On Mon, Dec 08, 2014 at 10:46:44AM +0100, Hans de Goede wrote:
> We use 2 mechanisms to unregister the trackpoint event listener depending on
> device removal order.
> 
> 1) We have a device_removed callback, if the trackpoint gets removed before
> the touchpad, this gets called, sees the device being removed is the trackpoint
> and unregisters the listener
> 
> 2) If the touchpad gets removed first, then in tp_destroy we unregister the
> listener
> 
> 2) May be delayed beyond the destruction of the trackpoint itself if the
> libinput user has a reference to the libinput_device for the touchpad.
> When this happens the trackpoint still has an eventlistener at destroy time
> and an assert triggers.
> 
> To fix this we must do 2) at the same time as we do 1), so at remove time.
> 
> While working on this I noticed that the touchpad code was also cancelling
> timers at destroy time rather then remove time, which means that they may
> expire between remove and destroy time, and cause events to be emitted from
> a removed device, so this commit moves the cancelling of the timers to the
> remove callback as well.
> 
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>

Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net> for both but see the
comment about calling evdev_suspend first.

Cheers,
   Peter

> ---
>  src/evdev-mt-touchpad-buttons.c     |  2 +-
>  src/evdev-mt-touchpad-edge-scroll.c |  2 +-
>  src/evdev-mt-touchpad-tap.c         |  2 +-
>  src/evdev-mt-touchpad.c             | 24 ++++++++++++++++--------
>  src/evdev-mt-touchpad.h             |  6 +++---
>  5 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/src/evdev-mt-touchpad-buttons.c b/src/evdev-mt-touchpad-buttons.c
> index cfd6588..6af3fcf 100644
> --- a/src/evdev-mt-touchpad-buttons.c
> +++ b/src/evdev-mt-touchpad-buttons.c
> @@ -603,7 +603,7 @@ tp_init_buttons(struct tp_dispatch *tp,
>  }
>  
>  void
> -tp_destroy_buttons(struct tp_dispatch *tp)
> +tp_remove_buttons(struct tp_dispatch *tp)
>  {
>  	struct tp_touch *t;
>  
> diff --git a/src/evdev-mt-touchpad-edge-scroll.c b/src/evdev-mt-touchpad-edge-scroll.c
> index d68fc68..3684c8a 100644
> --- a/src/evdev-mt-touchpad-edge-scroll.c
> +++ b/src/evdev-mt-touchpad-edge-scroll.c
> @@ -271,7 +271,7 @@ tp_edge_scroll_init(struct tp_dispatch *tp, struct evdev_device *device)
>  }
>  
>  void
> -tp_destroy_edge_scroll(struct tp_dispatch *tp)
> +tp_remove_edge_scroll(struct tp_dispatch *tp)
>  {
>  	struct tp_touch *t;
>  
> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
> index a38edbe..c34b203 100644
> --- a/src/evdev-mt-touchpad-tap.c
> +++ b/src/evdev-mt-touchpad-tap.c
> @@ -720,7 +720,7 @@ tp_init_tap(struct tp_dispatch *tp)
>  }
>  
>  void
> -tp_destroy_tap(struct tp_dispatch *tp)
> +tp_remove_tap(struct tp_dispatch *tp)
>  {
>  	libinput_timer_cancel(&tp->tap.timer);
>  }
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 5af0062..5938080 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -517,9 +517,9 @@ tp_stop_scroll_events(struct tp_dispatch *tp, uint64_t time)
>  }
>  
>  static void
> -tp_destroy_scroll(struct tp_dispatch *tp)
> +tp_remove_scroll(struct tp_dispatch *tp)
>  {
> -	tp_destroy_edge_scroll(tp);
> +	tp_remove_edge_scroll(tp);
>  }
>  
>  static void
> @@ -673,7 +673,7 @@ tp_process(struct evdev_dispatch *dispatch,
>  }
>  
>  static void
> -tp_destroy_sendevents(struct tp_dispatch *tp)
> +tp_remove_sendevents(struct tp_dispatch *tp)
>  {
>  	libinput_timer_cancel(&tp->sendevents.trackpoint_timer);
>  
> @@ -683,15 +683,23 @@ tp_destroy_sendevents(struct tp_dispatch *tp)
>  }
>  
>  static void
> +tp_remove(struct evdev_dispatch *dispatch)
> +{
> +	struct tp_dispatch *tp =
> +		(struct tp_dispatch*)dispatch;
> +
> +	tp_remove_tap(tp);
> +	tp_remove_buttons(tp);
> +	tp_remove_sendevents(tp);
> +	tp_remove_scroll(tp);
> +}
> +
> +static void
>  tp_destroy(struct evdev_dispatch *dispatch)
>  {
>  	struct tp_dispatch *tp =
>  		(struct tp_dispatch*)dispatch;
>  
> -	tp_destroy_tap(tp);
> -	tp_destroy_buttons(tp);
> -	tp_destroy_sendevents(tp);
> -	tp_destroy_scroll(tp);
>  
>  	free(tp->touches);
>  	free(tp);
> @@ -863,7 +871,7 @@ tp_tag_device(struct evdev_device *device,
>  
>  static struct evdev_dispatch_interface tp_interface = {
>  	tp_process,
> -	NULL, /* remove */
> +	tp_remove,
>  	tp_destroy,
>  	tp_device_added,
>  	tp_device_removed,
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index da9c091..1450288 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -287,7 +287,7 @@ int
>  tp_init_tap(struct tp_dispatch *tp);
>  
>  void
> -tp_destroy_tap(struct tp_dispatch *tp);
> +tp_remove_tap(struct tp_dispatch *tp);
>  
>  int
>  tp_init_buttons(struct tp_dispatch *tp, struct evdev_device *device);
> @@ -298,7 +298,7 @@ tp_init_softbuttons(struct tp_dispatch *tp,
>  		    double topbutton_size_mult);
>  
>  void
> -tp_destroy_buttons(struct tp_dispatch *tp);
> +tp_remove_buttons(struct tp_dispatch *tp);
>  
>  int
>  tp_process_button(struct tp_dispatch *tp,
> @@ -338,7 +338,7 @@ int
>  tp_edge_scroll_init(struct tp_dispatch *tp, struct evdev_device *device);
>  
>  void
> -tp_destroy_edge_scroll(struct tp_dispatch *tp);
> +tp_remove_edge_scroll(struct tp_dispatch *tp);
>  
>  void
>  tp_edge_scroll_handle_state(struct tp_dispatch *tp, uint64_t time);
> -- 
> 2.1.0
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>