Skip to content

Commit

Permalink
platforms/evdev: Clean up mir::Device handle on device remove (#3150)
Browse files Browse the repository at this point in the history
Not all `ConsoleProvider`s will send a `removed` event on the `DeviceObserver`
(in fact, only logind will), so we can't rely on that to clean up the handle
in `device_watchers`.

Furthermore, libinput itself will notice a device going away (by monitoring
the fd) and generate `LIBINPUT_EVENT_DEVICE_REMOVED`, which we might process
*before* the udev `REMOVED` event, and in *that* case, `device_removed` will
have already deleted the `LibInputDevice` and so the udev event handler
will *also* not clean up `device_watchers`.

Resolve this by cleaning up `device_watchers` *only* in `device_removed`,
and rely on the `DeviceObserver::removed` codepath triggering a libinput
removal event.

Fixes: #3149
  • Loading branch information
RAOF authored Nov 24, 2023
1 parent 7426ea9 commit 5205713
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions src/platforms/evdev/platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,7 @@ class InputDeviceObserver : public mir::Device::Observer
action_queue->enqueue(
[
&devices = devices,
syspath = syspath,
&device_watchers = device_watchers,
devnum = devnum
syspath = syspath
]()
{
for (auto const& input_device : devices)
Expand All @@ -229,7 +227,6 @@ class InputDeviceObserver : public mir::Device::Observer
}
}
}
device_watchers.erase(devnum);
});
}

Expand All @@ -238,9 +235,7 @@ class InputDeviceObserver : public mir::Device::Observer
action_queue->enqueue(
[
&devices = devices,
syspath = syspath,
&device_watchers = device_watchers,
devnum = devnum
syspath = syspath
]()
{
for (auto const& input_device : devices)
Expand All @@ -255,7 +250,6 @@ class InputDeviceObserver : public mir::Device::Observer
}
}
}
device_watchers.erase(devnum);
});
}

Expand Down Expand Up @@ -352,7 +346,6 @@ void mie::Platform::start()
if (strcmp(device.syspath(), udev_device_get_syspath(device_udev)) == 0)
{
libinput_path_remove_device(input_device->device());
device_watchers.erase(device.devnum());
}
}
}
Expand Down Expand Up @@ -453,8 +446,18 @@ void mie::Platform::device_removed(libinput_device* dev)
if (known_device_pos == end(devices))
return;

auto const udev_device = libinput_device_get_udev_device(dev);
input_device_registry->remove_device(*known_device_pos);
devices.erase(known_device_pos);
if (udev_device)
{
/* libinput documents that input devices might not have udev devices
* In practise all our devices *will*, because we only add devices from udev events,
* but let's not crash if not!
*/
device_watchers.erase(udev_device_get_devnum(udev_device));
udev_device_unref(udev_device);
}

log_info("Removed %s", describe(dev).c_str());
}
Expand Down

0 comments on commit 5205713

Please sign in to comment.