Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[VT console provider] Input device not recognised after disconnect/reconnect cycle #3149

Closed
RAOF opened this issue Nov 24, 2023 · 2 comments · Fixed by #3150
Closed

[VT console provider] Input device not recognised after disconnect/reconnect cycle #3149

RAOF opened this issue Nov 24, 2023 · 2 comments · Fixed by #3150
Labels
bug

Comments

@RAOF
Copy link
Contributor

RAOF commented Nov 24, 2023

When using --console=vt, my USB keyboard works both if it is plugged in before Mir is started or if it is plugged in after Mir has started.

However, when it is unplugged, Mir (correctly) notices it has disappeared, but never notices when it is plugged back in.

This is specific to the LinuxVirtualTerminal console provider; when using logind I can plug and unplug the device multiple times and it works as expected.
@RAOF
Copy link
Contributor Author

RAOF commented Nov 24, 2023

Aha! This is because in evdev/platform.cpp we rely on the ConsoleProvider to call the removed() method on the InputDeviceObserver when a device is removed, but only the LogindConsoleProvider actually does this.

For LinuxVirtualTerminal, failure to call removed() means that on hot-unplug the input device is removed, but the corresponding mir::Device is not removed from device_watchers, and so when plugged back in our udev ADDED event handler finds it in the device_watchers array, assumes we're already using it, and bails.

RAOF added a commit that referenced this issue Nov 24, 2023
@RAOF
platforms/evdev: Clean up mir::Device handle on device remove
1663f61 
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
@RAOF RAOF mentioned this issue Nov 24, 2023
Merged
RAOF added a commit that referenced this issue Nov 24, 2023
@RAOF
platforms/evdev: Clean up mir::Device handle on device remove
d18dec7 
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
@AlanGriffiths
Copy link
Contributor

Thanks for tracking this down. I can now reproduce

AlanGriffiths pushed a commit that referenced this issue Nov 24, 2023
@RAOF
platforms/evdev: Clean up mir::Device handle on device remove (#3150)
5205713 
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
AlanGriffiths pushed a commit that referenced this issue Nov 24, 2023
@RAOF @AlanGriffiths
platforms/evdev: Clean up mir::Device handle on device remove (#3150)
65e4d48 
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
(cherry picked from commit 5205713)
@AlanGriffiths AlanGriffiths mentioned this issue Dec 1, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

platforms/evdev: Clean up mir::Device handle on device remove canonical/mir
2 participants
@RAOF @AlanGriffiths