[PATCH libinput 0/1] Initialization race against udev

[Pekka Paalanen]

From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

Hi,

here is a patch for a race that was troubling a distribution for an
embedded device. The original form of this patch has been in developer
use for months now, and it seems to work there. For this version I have
only changed the logging and fixed a leak.

I originally discussed this issue on #wayland with Peter Hutterer on
September 22, 2017. The discussion is quoted below to refresh our
memories.

In the distribution, Weston is started by systemd, the system is not
very CPU-powerful, and lots of things are happening during the boot,
which may all contribute to making this race possible to lose.

I've only tested briefly on my work desktop to see that Weston still
appears to find the input devices I expect. Obviously my desktop would
never lose the race, because there are no input devices being hotplugged
at the same time as Weston is starting up.

The timer/idle callback idea is not implemented here, and neither is the
double-add filtering. Let me know if you require those.


Thanks,
pq


< pq> whot, btw. we've looking into some fun libinput vs. udev
device initialization race here. Apparently the device enumeration
on libinput start-up may race with udev preparing devices at the
same time, so the initial device enumeration for weston may see
devices that have not had all their udev properties set yet. Do you
recall ever fighting such issues?

< pq> whot, another issue is that we may see a double-add of a
device, first from the initial enumeration (with possibly missing
properties) and then a second time as a hotplug event because
libinput (correctly) listens for events before it enume existing
devices.

< whot> pq: yes, I've seen this a lot with the test suite, but never
in real life

< whot> I'm pretty sure in my case it's always a lingering udev
device or some lingering properties

< whot> e.g. a tablet has some keyboard properties set because the
kernel re-uses the event node

< whot> but that's triggered by the test suite using the path
interface and there's bound to be a window where we can race. it
shouldn't happen with the udev device, I think

< pq> specifically, in the target device/system with touchscreens
and weston, there are udev rules to set WL_OUTPUT on the device, but
weston does not always see it.

< pq> well, we use libinput via weston, and I don't think weston
uses the path-based function. It's really the initial enumeration of
input devices in libinput that has the issue.

< whot> pq: a double-add is a bug, I can picture that happening with
the current code. should be fixable

 * whot mumples something about syspath comparisons to filter that
out

< whot> mumbles, even. but it's late nough that I'm also open for
mumpling

< pq> whot, nice. Yeah, let's see if we have a patch to submit or
just a bug report, it might take a while.

< whot> pq: for the missing properties I have no idea, but that
would, if anything, be a systemd bug

< pq> whot, really? What's udev_device_get_is_initialised() for
then? Does it not apply to the initial device enumeration as done by
libinput?

< pq> whot, how does initial device enumeration work? Does it go the
udev daemon or just look in sysfs on its own?

< whot> udev_device_get_is_initialized may help but it didn't with
the test suite

< whot> but for your case, it might just because you're fighting a
different race

< whot> should be easy enough to printf that to the log in your case
and check if it's false when it fails

< pq> whot, oh yeah, this didn't feel like a lingering device issue.

< whot> initial enumeration: see udev_input_add_devices in
src/udev-seat.c

< whot> basically: "get list of devices maching subsystem input,
create udev device for each syspath in that list"

< pq> whot, our current idea is to have the initial enumeration to
ignore devices where udev_device_get_is_initialized() returns false,
because presumably we will get the hotplug event later anyway.

< pq> that would avoid any busy-loop waiting

< whot> you could schedule a callback for that device's syspath

< whot> takes the whole "presumably" out of the question :)

< pq> you mean like an idle task to re-check it?

< whot> yeah, that or a timer

< whot> not sure we have idle sources in libinput right now, so a
timer is your better option here

< pq> whot, ok, sure. Thanks for the tip. :-)

Nandor Han (1):
  udev: validate input devices during cold-plug

 src/udev-seat.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

-- 
2.16.1