From a6f0e4ae60177e792c0b0b75004368712b87365e Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Tue, 19 Sep 2017 15:51:09 +1000 Subject: [PATCH] timer: flush the timer funcs if our events come in late Avoid processing an event with a time later than the earliest timer expiry. If libinput_dispatch() isn't called frequently enough, we may have e.g. a tap timeout happening but read a subsequent input event first. In that case we can erroneously trigger or miss out on taps, see wrong palm detection, etc. Signed-off-by: Peter Hutterer Reviewed-by: Benjamin Tissoires --- src/evdev.c | 1 + src/libinput-private.h | 1 + src/timer.c | 62 +++++++++++++++++++++++++---------- src/timer.h | 3 ++ test/litest.c | 14 ++++++++ test/litest.h | 4 +++ test/test-misc.c | 73 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 141 insertions(+), 17 deletions(-) diff --git a/src/evdev.c b/src/evdev.c index 12ee4882..76006da8 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -2330,6 +2330,7 @@ evdev_process_event(struct evdev_device *device, struct input_event *e) libevdev_event_code_get_name(e->type, e->code), e->value); #endif + libinput_timer_flush(evdev_libinput_context(device), time); dispatch->interface->process(dispatch, device, e, time); } diff --git a/src/libinput-private.h b/src/libinput-private.h index 1a564f9f..a59cb08e 100644 --- a/src/libinput-private.h +++ b/src/libinput-private.h @@ -117,6 +117,7 @@ struct libinput { struct list list; struct libinput_source *source; int fd; + uint64_t next_expiry; } timer; struct libinput_event **events; diff --git a/src/timer.c b/src/timer.c index a136b8d5..6dc2f8b2 100644 --- a/src/timer.c +++ b/src/timer.c @@ -74,6 +74,8 @@ libinput_timer_arm_timer_fd(struct libinput *libinput) r = timerfd_settime(libinput->timer.fd, TFD_TIMER_ABSTIME, &its, NULL); if (r) log_error(libinput, "timer: timerfd_settime error: %s\n", strerror(errno)); + + libinput->timer.next_expiry = earliest_expire; } void @@ -125,24 +127,9 @@ libinput_timer_cancel(struct libinput_timer *timer) } static void -libinput_timer_handler(void *data) +libinput_timer_handler(struct libinput *libinput , uint64_t now) { - struct libinput *libinput = data; struct libinput_timer *timer; - uint64_t now; - uint64_t discard; - int r; - - r = read(libinput->timer.fd, &discard, sizeof(discard)); - if (r == -1 && errno != EAGAIN) - log_bug_libinput(libinput, - "timer: error %d reading from timerfd (%s)", - errno, - strerror(errno)); - - now = libinput_now(libinput); - if (now == 0) - return; restart: list_for_each(timer, &libinput->timer.list, link) { @@ -168,6 +155,28 @@ restart: } } +static void +libinput_timer_dispatch(void *data) +{ + struct libinput *libinput = data; + uint64_t now; + uint64_t discard; + int r; + + r = read(libinput->timer.fd, &discard, sizeof(discard)); + if (r == -1 && errno != EAGAIN) + log_bug_libinput(libinput, + "timer: error %d reading from timerfd (%s)", + errno, + strerror(errno)); + + now = libinput_now(libinput); + if (now == 0) + return; + + libinput_timer_handler(libinput, now); +} + int libinput_timer_subsys_init(struct libinput *libinput) { @@ -180,7 +189,7 @@ libinput_timer_subsys_init(struct libinput *libinput) libinput->timer.source = libinput_add_fd(libinput, libinput->timer.fd, - libinput_timer_handler, + libinput_timer_dispatch, libinput); if (!libinput->timer.source) { close(libinput->timer.fd); @@ -199,3 +208,22 @@ libinput_timer_subsys_destroy(struct libinput *libinput) libinput_remove_source(libinput, libinput->timer.source); close(libinput->timer.fd); } + +/** + * For a caller calling libinput_dispatch() only infrequently, we may have a + * timer expiry *and* a later input event waiting in the pipe. We cannot + * guarantee that we read the timer expiry first, so this hook exists to + * flush any timers. + * + * Assume 'now' is the current time check if there is a current timer expiry + * before this time. If so, trigger the timer func. + */ +void +libinput_timer_flush(struct libinput *libinput, uint64_t now) +{ + if (libinput->timer.next_expiry == 0 || + libinput->timer.next_expiry > now) + return; + + libinput_timer_handler(libinput, now); +} diff --git a/src/timer.h b/src/timer.h index 0190766a..828f506c 100644 --- a/src/timer.h +++ b/src/timer.h @@ -71,4 +71,7 @@ libinput_timer_subsys_init(struct libinput *libinput); void libinput_timer_subsys_destroy(struct libinput *libinput); +void +libinput_timer_flush(struct libinput *libinput, uint64_t now); + #endif diff --git a/test/litest.c b/test/litest.c index cb95a670..fb2c2c48 100644 --- a/test/litest.c +++ b/test/litest.c @@ -2931,6 +2931,20 @@ litest_is_motion_event(struct libinput_event *event) return ptrev; } +void +litest_assert_key_event(struct libinput *li, unsigned int key, + enum libinput_key_state state) +{ + struct libinput_event *event; + + litest_wait_for_event(li); + event = libinput_get_event(li); + + litest_is_keyboard_event(event, key, state); + + libinput_event_destroy(event); +} + void litest_assert_button_event(struct libinput *li, unsigned int button, enum libinput_button_state state) diff --git a/test/litest.h b/test/litest.h index 5868045c..0d133f35 100644 --- a/test/litest.h +++ b/test/litest.h @@ -653,6 +653,10 @@ litest_is_switch_event(struct libinput_event *event, enum libinput_switch sw, enum libinput_switch_state state); +void +litest_assert_key_event(struct libinput *li, unsigned int key, + enum libinput_key_state state); + void litest_assert_button_event(struct libinput *li, unsigned int button, diff --git a/test/test-misc.c b/test/test-misc.c index 788c4f98..32081c03 100644 --- a/test/test-misc.c +++ b/test/test-misc.c @@ -1419,6 +1419,78 @@ START_TEST(timer_offset_bug_warning) } END_TEST +START_TEST(timer_flush) +{ + struct libinput *li; + struct litest_device *keyboard, *touchpad; + + li = litest_create_context(); + + touchpad = litest_add_device(li, LITEST_SYNAPTICS_TOUCHPAD); + litest_enable_tap(touchpad->libinput_device); + libinput_dispatch(li); + keyboard = litest_add_device(li, LITEST_KEYBOARD); + libinput_dispatch(li); + litest_drain_events(li); + + /* make sure tapping works */ + litest_touch_down(touchpad, 0, 50, 50); + litest_touch_up(touchpad, 0); + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + + litest_assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + litest_assert_empty_queue(li); + + /* make sure dwt-tap is ignored */ + litest_keyboard_key(keyboard, KEY_A, true); + litest_keyboard_key(keyboard, KEY_A, false); + libinput_dispatch(li); + litest_touch_down(touchpad, 0, 50, 50); + litest_touch_up(touchpad, 0); + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY); + + /* Ingore 'timer offset negative' warnings */ + litest_disable_log_handler(li); + + /* now mess with the timing + - send a key event + - expire dwt + - send a tap + and then call libinput_dispatch(). libinput should notice that + the tap event came in after the timeout and thus acknowledge the + tap. + */ + litest_keyboard_key(keyboard, KEY_A, true); + litest_keyboard_key(keyboard, KEY_A, false); + litest_timeout_dwt_long(); + litest_touch_down(touchpad, 0, 50, 50); + litest_touch_up(touchpad, 0); + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + litest_restore_log_handler(li); + + litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_PRESSED); + litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_RELEASED); + litest_assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_PRESSED); + litest_assert_button_event(li, BTN_LEFT, + LIBINPUT_BUTTON_STATE_RELEASED); + + litest_delete_device(keyboard); + litest_delete_device(touchpad); + libinput_unref(li); +} +END_TEST + void litest_setup_tests_misc(void) { @@ -1438,6 +1510,7 @@ litest_setup_tests_misc(void) litest_add_no_device("config:status string", config_status_string); litest_add_for_device("timer:offset-warning", timer_offset_bug_warning, LITEST_SYNAPTICS_TOUCHPAD); + litest_add_no_device("timer:flush", timer_flush); litest_add_no_device("misc:matrix", matrix_helpers); litest_add_no_device("misc:ratelimit", ratelimit_helpers);