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

[Jonas Ådahl]

On Mon, Nov 23, 2015 at 12:01:01PM -0600, Derek Foreman wrote:
> 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?

Yes, sorry.

> 
> > 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? :(

The violation I speak of is that the object creation fails (violating
the rule that object creation never fails (if object creation is not
possible, the client is terminated).

So I'm not sure what would happen really. I suspect requests and
delete_id etc end up in /dev/null or something like that.

> 
> > 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...

Yes, a couple different attempts has been made to combat this. I think
Marek Chalupa had patches for a different approach.

> 
> > 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.

Not really, this doesn't change the seat lifetime management. A seat
going away is a separate problem. A client can still remove wl_pointer's
and wl_seat's just as before, it'd only affect whether a wl_pointer on a
wl_seat would suddenly become active again, given the state of the seats
capability set.


Jonas

> 
> 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
> > 
>