additional race in the wayland protocols?

[Eugen Friedrich]

>
> On Tue, 26 Mar 2019 22:05:28 +0100
> Eugen Friedrich <friedrix at gmail.com> wrote:
>
> > >
> > > On Mon, 25 Mar 2019 22:08:38 +0100
> > > Eugen Friedrich <friedrix at gmail.com> wrote:
> > >
>
> > > > > I would also like to see if we can forbid requests that both destroy
> > > > > the protocol object and create another one in one message. This would
> > > > > avoid the need for wl_proxy_marshal_constructor_destroy(). Marshalling
> > > > > is already getting a little bit combinatorial with constructor,
> > > > > versioned, and array.
> > > > >
> > > > this I did not get!
> > > > the requests are not in the same message, destroy is an request
> > > > and create an event in this case, also triggered by different calls:
> > > > wl_display_flush for sending requrest and
> > > > wl_display_read/dispatch* to receive the events.
> > > > Have nothing in mind how to prevent the race, what should be forbidden?
> > >
> > > I wasn't talking about any known existing protocol extension we know
> > > of. It is just that I suspect it is currently not forbidden to design a
> > > request that is both a destructor and has a new_id argument, so we
> > > should assume that someone out there is doing exactly that.
> > >
> > > If someone is doing that, wayland-scanner needs to figure out how to
> > > call the wl_proxy API. To make that use case race-free against
> > > everything else that might be going on, there would need to be a
> > > wl_proxy_marshal_constructor_destroy() kind of functionality. I would
> > > not like to have to add that, it seems too niche.
> > >
> > > If it was added, it would need versioned vs. unversioned, and array vs.
> > > vararags forms as well, so it would be at least four new ABI functions.
> > >
> > maybe it will be possible to forbid any arguments for type=destructor?
> > at least currently could not find any desctructor with arguments in
> > the wayland-protocols..
>
> Arguments in destructors are not problematic per se, it's only the
> new_id arguments.
>
> If we went for forbidding either, first would need to check how
> wayland-scanner handles the case right now. If it is obviously not
> working, then we can just make it more explicit and intentional with
> nothing to worry about. If it looks like it may work, then we need a
> deprecation period with wayland-scanner warning or failing on it to see
> if anyone was actually using it. We also need a backup plan in case we
> notice it is something people use and we cannot forbid.
>
> There are lots of extensions outside of wayland-protocols, a lot more
> than in wayland-protocols.
>
I took a look to the scanner and the destructors with arguments and
also new_id arguments are technically possible, scanner can handle it
and generate the code.
also I think I found a use-case where it even can be useful by maybe
redesigning a bit the protocol :-):
in the "linux-dmabuf-unstable-v1.xml" protocol the "create_immed"
request creates a buffer and it could just destroy the param proxy
within one request, so i guess even described use case is not a
completely valid one there might be some protocols which could
implement similar behavior.

Actually while playing around with the scanner generated code I found
out that even now to implement the race free destroying we need to add
a family of new API's in wayland-client.so if arguments are supported:
some marshal_create*_destroy family. to avoid this i see only two
other options:

1. hacky solution which was already proposed with proxy_wrapper-> ugly
2. fix the race only for destructors without parameters and print a
deprecate warning in case if destructor has some parameter, this we
should do if we agree to forbid parameters for destructors in the
future.

Thanks
eugen

>
> Thanks,
> pq