[RFC wayland 2/2] protocol: Don't silently fail to create seat device objects

[Jonas Ådahl]

On Thu, Jul 30, 2015 at 03:40:50PM +0200, Marek Chalupa wrote:
> On 07/30/2015 04:33 AM, Jonas Ådahl wrote:
> >One of the principles of Wayland is that object creation never fails. If
> >the object cannot be created, it should be reported as a protocol error
> >and an offending client should be terminated.
> >
> >A current violation to this principle is the input device creation in
> >wl_seat, as it may silently ignore a request if the set of seat
> >capabilities at the time of the request does not contain the capability
> >associated with the object that is to be created.
> >
> >This violation was introduced in the protocol in 53b57a63eeee06, and I
> >believe it may have been introduced in error, due to the fact that
> >weston implemented the violation.
> >
> >The interface specifies that "the request only takes effect if the seat
> >has the [given] capability", meaning if the seat does not advertise a
> >given capability, a request creating the corresponding object will fail
> >silently.
> >
> >This is racy, as a client may potentially create an object without
> >knowing whether it succeeded or not, and in effect may unknowingly never
> >receive any input events for the given capability.
> >
> >This is worked around in weston by always succeed if the capability has
> >ever been advertised. This goes against the protocol specification.
> >
> >This patch updates the protocol to get rid of the discussed race and the
> >protocol violation. The change should not cause any backward
> >compatibility issues, especially since "half" of the new semantics are
> >already implemented by for example weston.
> >
> >Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> >---
> >
> >While backward compatibility is not effected, it might be necessary to
> >make it explicit what version these semantics would be guaranteed so that
> >a client can rely on it knowing it won't create defunct objects. I'll let
> >this RFC out so it can be discussed whether this general change seems
> >like a desirable thing to do or not.
> >
> >
> >Jonas
> >
> >  protocol/wayland.xml | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> >diff --git a/protocol/wayland.xml b/protocol/wayland.xml
> >index 42c9309..eef95a7 100644
> >--- a/protocol/wayland.xml
> >+++ b/protocol/wayland.xml
> >@@ -1361,8 +1361,8 @@
> >          The ID provided will be initialized to the wl_pointer interface
> >  	for this seat.
> >
> >-        This request only takes effect if the seat has the pointer
> >-        capability.
> >+	This request always takes effect, but the created wl_pointer object
> >+	will only emit events as long as the seat has the pointer capability.
> >        </description>
> >        <arg name="id" type="new_id" interface="wl_pointer"/>
> >      </request>
> >@@ -1372,8 +1372,8 @@
> >          The ID provided will be initialized to the wl_keyboard interface
> >  	for this seat.
> >
> >-        This request only takes effect if the seat has the keyboard
> >-        capability.
> >+	This request always takes effect, but the created wl_keyboard object
> >+	will only emit events as long as the seat has the keyboard capability.
> >        </description>
> >        <arg name="id" type="new_id" interface="wl_keyboard"/>
> >      </request>
> >@@ -1383,8 +1383,8 @@
> >          The ID provided will be initialized to the wl_touch interface
> >  	for this seat.
> >
> >-        This request only takes effect if the seat has the touch
> >-        capability.
> >+	This request always takes effect, but the created wl_touch object
> >+	will only emit events as long as the seat has the touch capability.
> 
> Hi,
> 
> To have it comprehensive, you could mention that even the requests to such
> object will be ignored.
> 
> And more unspecified things: client should destroy the (let's say) wl_touch
> once it gets capabilities without touch (from the formulation
> is not clear that after the seat gains the touch capability back,
> the old object won't start functioning again).
> The same from the other side: what happens if client create wl_touch when it
> knows the seat does not have touch caps. From the new formulation it follows
> that it will end up with defunc object that will hang around until the
> client destroys it (it's client's fault anyway) - but as I wrote above, we
> should specify that such object won't start functioning once the touch
> capability is announced.

AFAIK nowhere in the protocol or the documentation does it say that a
wl_pointer (or the like) gets invalidated when the corresponding
capability is removed from the seat. In fact it is only mutter that does
this; if you don't recreate a wl_pointer in a client connected to
weston after a pointer is plugged out and then in again so that the
capability is removed and then added, the pointer resource will work
just as it did before.

I don't see any benefit in invalidating a previously valid input device
resource. If we want to stop violating the "create never fails"
principle, we still have to deal with the situation where we have
wl_pointer objects but no actual physical pointer device, so
invalidating it doesn't give us anything here really.

A benefit of not invalidating though is that we get rid of uncertainty
of validity. While clients tend to recreate the wl_pointer when it is
re-advertised, and with the current available protocols it is completely
harmless, it still doesn't feel like a good thing that a request
sent will affect the server side object differently given a server state
transition that the client will have no idea about when or if it will
occur.

I agree though that this behaviour needs to be clarified.


Jonas

> 
> >        </description>
> >        <arg name="id" type="new_id" interface="wl_touch"/>
> >      </request>
> >
> 
> Cheers,
> Marek