> > > > > On Wed, 27 Mar 2019 22:14:27 +0100 > > Eugen Friedrich <fried...@gmail.com> wrote: > > > > > > > > > > On Tue, 26 Mar 2019 22:05:28 +0100 > > > > Eugen Friedrich <fried...@gmail.com> wrote: > > > > > > > > > > > > > > > > On Mon, 25 Mar 2019 22:08:38 +0100 > > > > > > Eugen Friedrich <fried...@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. > > > > Hi Eugen, > > > > oh, you mean wl_proxy_marshal*_destroy()? That is exactly the thing > > needed to solve your original issue, some arguments or none. > > Hi Pekka, > > ach yes, currently I see a need for two new functions: > - destroying without and > - destroying with new_id arguments > > > > We shouldn't use marshal_constructor_destroy() as "maybe create". When a > > _constructor_ is called, there should be exactly one new_id argument. > > > > agree, also the generated code looks very different, in case we do not > have new_id > argument we would need to end up in returning NULL, ugly! > > > > to avoid this i see only two > > > other options: > > > > > > 1. hacky solution which was already proposed with proxy_wrapper-> ugly > > > > I believe that approach is illegal. The doc for > > wl_proxy_create_wrapper() says: > > > > * A proxy wrapper must be destroyed before the proxy it was created from. > > > > You would essentially be using a stale object ID to send out the > > request. If libwayland-client actually checked the validity of the ID, > > it would fail. > > > > ups, did not read carefully , but the workaround works at wayland 1.13 > for sure and should work on master (code review) > seams libwayland-client is not checking for validity, anyway I will > not mention this again, in any case it is a dirty workaround > > > > 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. > > > > The fix for destructors without parameters at all will also fix > > destructors with parameters that are not new_id. > > > > Only the new_id case would need yet more new API. > > > > I see, will then add those two and see how to carefully implement this > in scanner.c > > > *** > > > > A wild idea I have not investigated at all would be to add the > > destructor annotation into the message signature string. Then we could > > make the existing marshall functions handle the destruction as well. It > > might be tricky to pull off in a reliable backwards compatible manner, > > but I don't think it would be impossible. > > > > The new_id args are already in the message signature, obviously, so i > > we wanted, we could collapse marshal and marshal_constructor calls into > > one marshal_maybe_construct function. > > > > If we did both, then in the end we would only have: > > - wl_proxy_destroy() for destroy without a request > > - wl_proxy_marshal_maybe_construct{,_array} for everything else > > > > I'm not sure that is worth though, given that the old functions must > > remain and keep working. > > > > doesn't look like good solution to me: > - code generation is different if we have a new_id arguments, we > returning a new object in this case > - currently "destroy" request is just another type of message and not > reflected anywhere else, scanner is checking for it explicitly and it > is allowed to have many destructors. > Scanner checks only if request with the name destroy is of type destructor. > To use you "wild idea" from above we would need to "miss use" > arguments signature for message type and take of it correctly in all > of the places in the wayland-client code. > Backwards compatibility should be possible but in general I would not > follow this solution, at least for now. > will continue with two new API's... >
Hi Pekka, I revisited my opinion about you suggestion and changed my mind. will not tell here why exactly, basically I just tried my and you approach. You solution is quite elegant and I saw that signature is already use for "since" description so the wayland code is prepared to deal with some chars in signature which are not argument. I will suggest this solution (destroy "tag" in signature, so far I choose "-" and I adding this as last attribute) in next days. best regard jeka > best regards > jeka > > > > Thanks, > > pq _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel