[PATCH libinput] timer: flush the timer funcs if our events come in late

Peter Hutterer peter.hutterer at who-t.net
Wed Sep 20 00:20:08 UTC 2017


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 <peter.hutterer at who-t.net>
---
 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 e209ae0d..c92bd8b6 100644
--- a/src/evdev.c
+++ b/src/evdev.c
@@ -825,6 +825,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 1f238431..17b8775a 100644
--- a/test/litest.c
+++ b/test/litest.c
@@ -2930,6 +2930,20 @@ litest_is_motion_event(struct libinput_event *event)
 }
 
 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 679406dc..61a27fe3 100644
--- a/test/litest.h
+++ b/test/litest.h
@@ -653,6 +653,10 @@ litest_is_switch_event(struct libinput_event *event,
 		       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,
 			   enum libinput_button_state state);
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);
-- 
2.13.5



More information about the wayland-devel mailing list