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

[Peter Hutterer]

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.

Cheers,
   Peter