[PATCH v2 weston] input: Don't test keyboard/pointer/touch pointers

[Daniel Stone]

Hi,

On 2 February 2015 at 22:26, Hardening <rdp.effort at gmail.com> wrote:
> Le 02/02/2015 22:24, Derek Foreman a écrit :
>> On 02/02/15 01:40 PM, Bill Spitzak wrote:
>>> Is there a reason it does not just clear the pointer when
>>> keyboard_device_count is changed to zero? That would seem like a smaller
>>> patch.
>>
>> Pulled Jonas Ådahl in on the CC for that question...  I understand why
>> weston_pointer isn't freed when released, but is there a need for the
>> same behavior for weston_keyboard and weston_touch?
>
> I had a discussion on this on IRC, and the reason is that we wanna save
> the last position of the pointer. I'm not sure it's a valid argument, as
> the counter part is that the cleaning of pointer, keyboard and touch is
> not done, or done partially. Not so nice for weston, a reference
> implementation, the code should be straightforward and here it's not.

There's another reason, which is that I misdesigned the wl_seat
protocol, and it has a race around sub-seat destruction of
keyboard/pointer/touch objects. If we remove the objects when they
actually disappear, we'll incorrectly kill clients in the following
scenario, which uses keyboard as an example but could equally be
pointer or touch:
  - client binds to wl_seat
  - compositor sends wl_seat::capabilities(KEYBOARD)
  - keyboard is unplugged, compositor destroys seat->keyboard
  - client calls wl_seat::get_keyboard()
  - oops ...

The easiest way to deal with this is to never destroy
seat->{keyboard,pointer,touch}. A cleaner alternative would be to
detect a client binding to these and send objects which are
immediately destroyed, which I'd happily review patches for, but in
the meantime we should leave the current behaviour.

Per earlier discussion, wl_seat also needs a bump to add a release
event, mind. When this is done, we could correct the historical
mistake and add direct events from wl_seat which add
new_id(keyboard|pointer|touch) methods rather than the current
race-prone capabilities/get_* dance.

Cheers,
Daniel