[PATCH] Touch: Fix duplicate TouchBegin selection with virtual devices

[Chase Douglas]

On Sat, Sep 29, 2012 at 4:00 PM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On 30/09/12 04:25 , Chase Douglas wrote:
>>
>> On Thu, Sep 27, 2012 at 11:34 PM, Peter Hutterer
>> <peter.hutterer at who-t.net> wrote:
>>>
>>> On Fri, Sep 07, 2012 at 06:30:23PM +0100, Daniel Stone wrote:
>>>>
>>>> Given the following scenario:
>>>>    1) client A selects for TouchBegin on window W for device D
>>>>    2) client B selects for TouchBegin on window W for XIAllDevices
>>>>    3) client C selects for TouchBegin on window W with device E
>>>>
>>>> Step 3 will fail with BadImplementation, because attempting to look up
>>>> XIAllDevices or XIAllMasterDevices with dixLookupDevices doesn't work.
>>>> This should succeed (or, if it was selecting for device D, fail with
>>>> BadAccess as it would be a duplicate selection).
>>>>
>>>> Fix this by performing the appropriate lookup for virtual devices.
>>>>
>>>> Signed-off-by: Daniel Stone <daniel at fooishbar.org>
>>>> Cc: Peter Hutterer <peter.hutterer at who-t.net>
>>>> Cc: Chase Douglas <chase.douglas at ubuntu.com>
>>>> ---
>>>>   Xi/xiselectev.c |    9 +++++++--
>>>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Xi/xiselectev.c b/Xi/xiselectev.c
>>>> index 0e45cb8..ab1b624 100644
>>>> --- a/Xi/xiselectev.c
>>>> +++ b/Xi/xiselectev.c
>>>> @@ -180,8 +180,13 @@ ProcXISelectEvents(ClientPtr client)
>>>>                       if (CLIENT_ID(iclient->resource) == client->index)
>>>>                           continue;
>>>>
>>>> -                    dixLookupDevice(&tmp, evmask->deviceid,
>>>> serverClient,
>>>> -                                    DixReadAccess);
>>>> +                    if (evmask->deviceid == XIAllDevices)
>>>> +                        tmp = inputInfo.all_devices;
>>>> +                    else if (evmask->deviceid == XIAllMasterDevices)
>>>> +                        tmp = inputInfo.all_master_devices;
>>>> +                    else
>>>> +                        dixLookupDevice(&tmp, evmask->deviceid,
>>>> serverClient,
>>>> +                                        DixReadAccess);
>>>>                       if (!tmp)
>>>>                           return BadImplementation;       /* this
>>>> shouldn't happen */
>>>>
>>>> --
>>>> 1.7.10.4
>>>
>>>
>>> We may have to document the protocol better.
>>>
>>> The protocol states that if there is overlapping selection, we must
>>> return
>>> BadAccess. With the patch above added, we get BadAccess if the first
>>> client
>>> registers for XIAll(Master)Devices and the second client registers for
>>> anything. But the inverse, the first client registering for a device and
>>> the
>>> second client registering for XIAll(Master)Devices succeeds.
>>>
>>> Now, I think this behaviour makes sense but it needs to be documented
>>> explicitely. Plus, we'd have to test that the server actually behaves
>>> that
>>> way, because I'm pretty sure right now it doesn't.
>>
>>
>> This sounds dangerous. It could lead to subtle race conditions where
>> behavior depends on which client request reaches the server first.
>>
>> Why does this behavior seem better to you than a strict mutual exclusion
>> policy?
>
>
> my email wasn't clear enough, sorry. I think the behaviour of letting one
> client select on XIAll* and one client select on a specific device makes
> sense, and it should work regardless of who selects what first.
>
> right now (with this patch), only one direction works and that's definitely
> wrong behaviour.

Sorry for the delay, I've been trying to think about this to come up
with a cogent argument.

I don't like the idea of overlapping selections being allowed. There
are two ways to handle an overlapping selection:

* Only deliver to one of the selections. Which do you pick? Will the
unpicked client be annoyed (i.e. will the developer be confounded when
his application fails to receive touch events, and is this a problem)?

* Deliver to both selections. This could cause multiple responses to a
single physical action, which I have always found troubling.

I don't think either is a great approach. I would prefer to lock it
down so there cannot be any overlap. Are there use cases that require
the ability for overlapping selections that I may not be aware of?

-- Chase