Destructor handling and safety about wayland-rs

[elinorbgr]

Okay.

Sorry it cannot end as well as you'd hope. To be honest, I'm not very happy with the C API exposed by the C wayland libraries, it already caused me much of a headache, because it's not very rust-friendly... I started working on it more than a year ago, and only now do I reach an API design that I feel is rust-friendly and low-overhead for both client and server side...

For now, I'll focus on releasing the version 0.7, and updating wayland-kbd, wayland-window, and the glutin/winit backend to the new design.

However, if you feel motivated enough to create a wlc-like crate on to of wayland_server, you can definitely ping me! I'd love to participate in such a project. 😉

[elinorbgr]

I think the third solution is the best one, it really depends on how significant this overhead is, and I have no idea how much it is.

[Timidger]

Yeah the destroyed flag seems to be the best solution.

Would it be possible to have the "worst of both world" so to speak, by providing raw bindings that would not check this flag, for those who would be willing to deal with it manually / aren't going to be dealing with objects that will be destroying themselves?

[elinorbgr]

Hmm... I don't see any way to do it except by providing two versions of all the libs.

Also, it appears after some quick evaluation that this overhead will not be very large compared to the raw cost of dispatching a message, so it probably would not be too much of a problem.

Or do you have other concerns than performance?

[Timidger]

If it's too much effort, then yeah it's not a huge concern especially if it's not actually a huge bottle neck.

[elinorbgr]

Hopefully the fix is en route.

Quick question @Timidger : what kind of interaction do you hope to make between wlc and wayland-server? Is it just declaring a few globals that will specifically be handled manually and letting the rest to wlc?

If it's just that, it'll be okay. but sharing wl_ressources between wayland-server and wlc will very likely be highly unsupported, as wayland-server relies a lot on the user_data field of wl_resource.

At least, there must be a clear distinction between ressources managed by wlc and ressource managed by wayland-server. (I don't know at all what wlc does tbh)

[Timidger]

Hmmm, one of the primary (and most immediate) use cases we had for the wayland ipc was to allow backgrounds to be managed by an external client. That way Way Cooler didn't have to worry about backgrounds, a different program could and we could simply draw the surface they give us.

This surface is picked up by wlc (as any other program's surface would) and is for the most parted treated as normal (i.e: managed by wlc). Of course, this is just a simple example. For more complex things we eventually want to implement (i.e redshift and potentially a graphical program similar to arandr) that story might change.

Pinging @SnirkImmington for his thoughts on this. I don't think it'll be a problem in general though, since we probably won't be messing with the internals of the wl_resources managed by wlc/wayland-server.

[SnirkImmington]

@vberger I'm not sure how much interaction we'll have between the two (aside from ipc) - wlc does management of wayland resources. I haven't read through the source but I think they manage lifetimes as well, as @Timidger said, with their wrappers wlc_resource and wlc_handle for example.

Our use cases mostly revolve around reading data off of a wl_resource. wlc provides a method to go from one of their handles to a wl_resource, (also as it happened the userdata field you mentioned, we have a wrapper for) and that's an opaque struct as far as rustwlc is concerned.

If we were to implement server-side borders, for example, we might want to get some information on a view that wlc doesn't provide, so we'll take some view handle and we'll get the underlying wl_resource - it would be nice to be able to use that without worrying about wayland-server's lifetime management - i.e. with the "unsafe/raw" methods @Timidger mentioned (especially because as far as wayland-server's lifetime management is concerned we'd be pulling them out of thin air).

We also wanted to have a crate feature in rustwlc for optional dependency on wayland-server, to allow things like that to work for end users.

We have our own considerations of "rewrite the whole thing to enforce Rust's memory model on it" - and we'd probably have different memory management concerns than wayland-server if we did. We put that rewrite on hold to get started with Way Cooler (so rustwlc has a more "unsafe" API right now) but we'd be willing to build up a "safety" layer that could integrate better with wayland-server). Perhaps if both libraries had an even divide between "raw bindings" and "Rust memory model" we could pass objects between the two more easily. I know that would end up being more work for you - let me know what you think/if what I'm saying makes sense.

[elinorbgr]

Integrating a way to create wayland objects "out of thin air", in order to give them as argument to event/requests is completely possible, but such objects cannot be registered to wayland-server event-loops, nor can their methods be called, or it'll either segfault or mess with the internal state (due to user_data being inconsistent with what the lib expects).

My goal with wayland-client/server is indeed to create an API that would be safe from a rust point of view, while being as close as possible to the original C API, and this with an overhead as small as possible. So I'm expect and hope that more high-level APIs can be build around these libs, and that's mostly their reason for existing.

[Timidger]

Perhaps we should clarify what we mean here by "out of thin air". For example, currently we construct our background image by writing to a buffer then attaching it to a surface (see here). Is our use of matching on the events triggered by this surface bad because we made this surface "out of thin air"? It's managed by wlc (because it's treated as a view, just like any other), but we are using wayland_sys's api here to display the correct pointer image.

EDIT: Or is it ok because it's just a client and so listening for these events, but the same can not be said for a server?

[elinorbgr]

Hmm, I'm not sure I get what you mean...

The main issue I see, is that, to safely do event dispatching & destructor handling, wayland-server (and similarly wayland-client) relies on data it stores via wl_resource_set_user_data() in the user_data pointer of the wl_resources. I don't know what wlc does with the resources, but it certainly does not do the same thing.

Now, imagine you've setup your compositor with wlc. You have clients connecting to it, and wlc creates wl_resource objects for you.

Now, you might want to have some of these object interact with code using wayland-server, and so create a wayland_server object, for example let's say a WlSurface.

You might want this WlSurface for different reasons:

• To register it to the EventLoop in wayland_server. This is only possible if you are sure that wlc will never ever try to do anything with the wl_surface, and especially not read its user_data or destroy it, overwise memory will get corrupted.
• To call one of its methods to send an event to the client. If wlc forfeits the object and wayland-server can overwrite its user_data, everything is fine. Otherwise, it can be safe-ish, as long as you don't call any destructor method, in which case wayland_server will try to clain the user_data to deallocate the memory associated.
• To give it as argument to a method of an object belonging to wayland_server. This is safe.

Now, there is a last edge-case. Imagine you have your "background setting protocol" (I'm completely speculating here), with a global object "background_setter", that expects a wl_buffer from the client to get the contents of the background.

Server-side, the wl_compositor is handled by wlc, so it's wlc that will be notified of the creation of the wl_buffer. Now, you use ayland-server for you background-setting protocol, so the request for setting the background will be dispatched to wayland-server's EventLoop. Internally, the C library will provide wayland-server with a *mut wl_resource corresponding to the wl_buffer associated with the event.

wayland-server has no way to know this wl_buffer isn't actually managed by it, but rather by wlc. So it'll just assume it's its, ans notably that its user_data has previously been initialized, and use it as such to provide a &WlBuffer to the handler method for the event...

After a quick glance, I can see that wlc also heavily uses the user_data field for its internal machinery... (see for example https://github.com/Cloudef/wlc/blob/master/src/compositor/compositor.c#L138 ).

Basically, I think it is only possible to mix wayland-server and wlc if they operate over completely different and non-intersecting protocols. Meaning, as you use wlc to handle the core protocol, that wayland_server must only handle ressources of a protocol extension that uses none of the core interfaces in any of its events...

I know all this digs into the wayland machinery, but I hope I'm clear?

[Timidger]

Thank you, you were very clear. I was not aware of how wlc managed its resource (for up until now we had not needed to know that), but it is clear now that Way Cooler will need to work out a different solution where we either ditch wlc (difficult) or work around this e.g with our own ipc system instead of wayland_sys.

[elinorbgr]

Well, using wayland_sys is perfectly fine for you, at least exactly as much as using directly the C libraries, as it's just raw bindings.

It's using wayland_server which is problematic...

[Timidger]

Yes sorry, I misspoke there, meant wayland_server, thanks.