[PATCH libinput 1/2] test: create a lock file to avoid parallel udev reloads during device add

[Jonas Ådahl]

On Tue, Jul 05, 2016 at 12:43:43PM +1000, Peter Hutterer wrote:
> litest_add_device and litest_delete_device trigger a udev rule reload. This
> messes with some test devices and when we run multiple tests in parallel we
> get weird errors like "keyboard $BLAH failed the touchpad sanity test".
> 
> Still not 100% reliable to run tests in parallel, but it's vastly improved
> now.
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>  configure.ac  |  3 +++
>  test/litest.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 66 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index 8c14efe..3e973d4 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -194,6 +194,9 @@ if test "x$build_tests" = "xyes"; then
>  		AC_DEFINE_UNQUOTED(HAVE_ADDR2LINE, 1, [addr2line found])
>  		AC_DEFINE_UNQUOTED(ADDR2LINE, ["$ADDR2LINE"], [Path to addr2line])
>  	fi
> +
> +	AC_DEFINE(LITEST_UDEV_LOCKFILE, ["/tmp/litest-udev.lock"],
> +		  [Lock file used to restrict udev reloads during tests])
>  fi
>  AM_CONDITIONAL(HAVE_LIBUNWIND, [test "x$HAVE_LIBUNWIND" = xyes])
>  
> diff --git a/test/litest.c b/test/litest.c
> index 9b4feed..1250b3f 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -1164,6 +1164,58 @@ litest_restore_log_handler(struct libinput *libinput)
>  	libinput_log_set_handler(libinput, litest_log_handler);
>  }
>  
> +static inline int
> +create_udev_lock_file(void)
> +{
> +	int lfd;
> +
> +	/* Running the multiple tests in parallel usually trips over udev
> +	 * not being  up-to-date. We change the udev rules for every device
> +	 * created, sometimes this means we end up getting the wrong udev
> +	 * device, or having wrong properties applied.
> +	 *
> +	 * litests use the path interface and there is a window between
> +	 * creating the device (which triggers udev reloads) and adding the
> +	 * device to the libinput context where another udev reload may
> +	 * upset things.
> +	 *
> +	 * To avoid this, create a lockfile on device add and device delete
> +	 * to make sure that we have exclusive access to udev while
> +	 * the udev rules are reloaded.
> +	 */
> +	do {
> +		lfd = open(LITEST_UDEV_LOCKFILE, O_CREAT|O_EXCL, O_RDWR);
> +
> +		if (lfd == -1) {
> +			struct stat st;
> +			time_t now = time(NULL);
> +
> +			litest_assert_int_eq(errno, EEXIST);
> +			msleep(10);
> +
> +			/* If the lock file is older than 10s, it's a
> +			   leftover from some aborted test */
> +			if (stat(LITEST_UDEV_LOCKFILE, &st) != -1) {
> +				if (st.st_mtime < now - 10) {

I suppose making sure the test case runs fine when changing between
summer time/not summer time is unimportant enough to care about, but I
suspect it could make this check fail.

This and the next one is Reviewed-by: Jonas Ådahl <jadahl at gmail.com>


Jonas

> +					fprintf(stderr,
> +						"Removing stale lock file %s.\n",
> +						LITEST_UDEV_LOCKFILE);
> +					unlink(LITEST_UDEV_LOCKFILE);
> +				}
> +			}
> +		}
> +	} while (lfd < 0);
> +
> +	return lfd;
> +}
> +
> +static inline void
> +delete_udev_lock_file(int lfd)
> +{
> +	close(lfd);
> +	unlink(LITEST_UDEV_LOCKFILE);
> +}
> +
>  struct litest_device *
>  litest_add_device_with_overrides(struct libinput *libinput,
>  				 enum litest_device_type which,
> @@ -1177,6 +1229,8 @@ litest_add_device_with_overrides(struct libinput *libinput,
>  	int rc;
>  	const char *path;
>  
> +	int lfd = create_udev_lock_file();
> +
>  	d = litest_create(which,
>  			  name_override,
>  			  id_override,
> @@ -1202,6 +1256,9 @@ litest_add_device_with_overrides(struct libinput *libinput,
>  		d->interface->min[ABS_Y] = libevdev_get_abs_minimum(d->evdev, ABS_Y);
>  		d->interface->max[ABS_Y] = libevdev_get_abs_maximum(d->evdev, ABS_Y);
>  	}
> +
> +	delete_udev_lock_file(lfd);
> +
>  	return d;
>  }
>  
> @@ -1258,9 +1315,13 @@ litest_handle_events(struct litest_device *d)
>  void
>  litest_delete_device(struct litest_device *d)
>  {
> +	int lfd;
> +
>  	if (!d)
>  		return;
>  
> +	lfd = create_udev_lock_file();
> +
>  	if (d->udev_rule_file) {
>  		unlink(d->udev_rule_file);
>  		free(d->udev_rule_file);
> @@ -1278,6 +1339,8 @@ litest_delete_device(struct litest_device *d)
>  	free(d->private);
>  	memset(d,0, sizeof(*d));
>  	free(d);
> +
> +	delete_udev_lock_file(lfd);
>  }
>  
>  void
> -- 
> 2.7.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel