[PATCH RFC xf86-input-libinput] Swap the registered input device on DEVICE_OFF when needed
Hans de Goede
hdegoede at redhat.com
Fri Oct 14 08:45:57 UTC 2016
Hi,
On 10/14/2016 09:13 AM, Peter Hutterer wrote:
> If we don't swap out the pInfo previously passed to xf86AddEnabledDevice(),
> the thread eventually calls read_input on a struct that has been deleted.
> Avoid this by swapping out the to-be-destroyed pInfo with the first one we
> find.
>
> FIXME: incomplete, this doesn't do any driver checking.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Hans: I'm not a big fan of copying the fields into our custom InputInfoRec,
> it makes the inner workings of xf86AddEnabledDevice() and the thread
> effectively input driver ABI. This one is a slightly nicer (yet incomplete)
> solution, what do you think?
I think your solution should work. But I like my solution more, as it seems
more KISS then yours, but I see your point about it relying on the server
side a bit too closely.
So I'll make it your call which fix you're going to go with.
Regards,
Hans
>
> src/xf86libinput.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index 69f7ae3..dcd3018 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -87,6 +87,7 @@
> struct xf86libinput_driver {
> struct libinput *libinput;
> int device_enabled_count;
> + void *registered_InputInfoPtr;
> };
>
> static struct xf86libinput_driver driver_context;
> @@ -583,6 +584,7 @@ xf86libinput_on(DeviceIntPtr dev)
> if (driver_context.device_enabled_count == 0) {
> #if HAVE_THREADED_INPUT
> xf86AddEnabledDevice(pInfo);
> + driver_context.registered_InputInfoPtr = pInfo;
> #else
> /* Can't use xf86AddEnabledDevice on an epollfd */
> AddEnabledDevice(pInfo->fd);
> @@ -1138,6 +1140,28 @@ xf86libinput_destroy(DeviceIntPtr dev)
> struct xf86libinput *driver_data = pInfo->private;
> struct xf86libinput_device *shared_device = driver_data->shared_device;
>
> + /* If the device being destroyed is the one we used for
> + * xf86AddEnabledDevice(), we need to swap it out for one that is
> + * still live. xf86AddEnabledDevice() buffers some data and once the
> + * deletes pInfo (when DEVICE_OFF completes) the thread will keep
> + * calling that struct's read_input because we never removed it.
> + * Avoid this by removing ours and substituting one that's still
> + * valid, the fd is the same anyway (libinput's epollfd).
> + */
> + if (pInfo == driver_context.registered_InputInfoPtr &&
> + driver_context.device_enabled_count > 0) {
> + InputInfoPtr next = xf86FirstLocalDevice();
> + while (next == pInfo)
> + next = pInfo->next;
> +
> + input_lock();
> + xf86RemoveEnabledDevice(pInfo);
> + if (next) /* shouldn't ever be NULL anyway */
> + xf86AddEnabledDevice(next);
> + driver_context.registered_InputInfoPtr = next;
> + input_unlock();
> + }
> +
> xorg_list_del(&driver_data->shared_device_link);
>
> if (driver_data->tablet_tool)
>
More information about the xorg-devel
mailing list