Lifetime of wl_seat objects (was Re: [PATCH wayland-protocols 1/2] Introduce wp_relative_pointer) interface

[Derek Foreman]

On 23/11/15 12:32 AM, Jonas Ådahl wrote:
> On Mon, Nov 23, 2015 at 02:42:09PM +1000, Peter Hutterer wrote:
>> On Fri, Nov 20, 2015 at 02:59:50PM +0800, Jonas Ådahl wrote:
>> [...]
>>>>> +    <request name="destroy" type="destructor">
>>>>> +      <description summary="release the relative pointer object"/>
>>>>> +    </request>
>>>>> +
>>>>
>>>> Should we add something about pairing this with wl_seat capabilities here?
>>>> Just looking at this protocol doesn't make it clear what happens if the
>>>> compositor drops the wl_pointer. Right now it's implicit, when the
>>>> capability drops you need to destroy the wl_pointer and the
>>>> wl_relative_pointer; spelling this out is useful.
>>>
>>> It's not even spelled out what happens to a wl_pointer when the seat
>>> looses the pointer capability. For example right now a client running on
>>> weston can ignore any capability lost event and just keep whatever
>>> objects it ever created and they'll work again. So, I think we should
>>> first make it clear there, then follow up here and do the same.
>>
>> urgh. so I guess we need to encode this in the protocol then, a
>> wl_pointer/keyboard/touch object once obtained is re-used in the seat when
>> the capability comes back. it's terrible IMO, but if we've been shipping
>> this behaviour there isn't much about this we can do now without breaking
>> existing clients. I don't mind writing the documentation patches, but I'd
>> like to have general consensus about it.

I guess it's worth pointing out that old clients aren't capable of doing
the right thing here, since we've only very recently added
wl_pointer_release() as opposed to wl_pointer_destroy().  :)

> It's a bit of a confusing mess, but IIRC this is the different cases
> that are implemented in mutter now:

I think you mean "weston" here, not mutter?

> 1. get object before first capability -> object not created (protocol
> violation)
> 
> 2. get object after first capability -> object created and valid for all
> eternity
> 
> 3. get object after capability is lost -> object will eventually work
> whenever the capability reappears

To the casual observer this looks dumb - but it's to prevent the race when:

server says seat has pointer caps
client says gimme pointer
device is unplugged before server receives request
(at this point if the objects aren't kept around forever the client gets
killed)

> mutter on the other hand does it slightly different. Case 1 is the same,
> but for case 2, the objects are forever defunct after the capability is
> lost. Case 3 works the same as case 1, i.e. the object is not created
> (protocol violation).

Does mutter kill apps when this race triggers? :(

> I'm not sure this is a good enough reason for doing the "capability lost
> -> defunct object", but some way or another we need to fix the spec and
> the implementations.
>
> Specifying it to create inert objects and "inerting" objects when a
> capability is lost will probably for the most part work, since all the
> example code has always destroyed and recreated objects as their
> corresponding capabilities disappear and reappear, but if there is some
> wild client that depends objects not going dark we'd break that.

I think David Fort was working on patches to actually do this...

> Specifying it to having the objects always be valid (but only certain
> effect, such as updating the pointer sprite, when applicable), clients
> that want to rely on this will only reliably work on new enough
> compositor versions, meaning to work on the current wl_seat inteface
> version, they'd still need to rely on the old undefined behaviour, to
> work across versions.

I like this less because if we let clients hang onto these input
resources forever then a seat can never go away.  Not a big deal for a
default single seat weston configuration, but for the rdp backend this
gets ugly (since a connection is a seat), it really wants to delete
seats when clients disconnect.

I suppose it would be possible to make the RDP backend re-use a single
seat for multiple client logins. :(

> Both these are not very optimal, and I think option 1 (create inert
> objects/inertify existing objects), while slightly ruthless, seems most
> sane, since it's what most if not all clients already expects. What we
> can't do, however, is to write the spec according to the weston
> implementation without making new clients depending on those semantics
> from not working on older versions of other compositors.

My vote is also for option 1.

I don't know how we dig out of this hole though.

How bad is it to have the wl_seat version dictate behavior, and any
version before the seat.*_release pointers existed needs to provide
pointers forever, any version newer can inert them or provide them as inert?

Not sure what the RDP backend could reasonably do - perhaps refuse to
support the old version of wl_seat?  (Can we even do that?)

> 
> Jonas
> 
>>
>> Cheers,
>>    Peter
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>