Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

destroy() Apis should be called on Drop #596

Open
A6GibKm opened this issue Jan 4, 2023 · 7 comments
Open

destroy() Apis should be called on Drop #596

A6GibKm opened this issue Jan 4, 2023 · 7 comments

Comments

@A6GibKm
Copy link

A6GibKm commented Jan 4, 2023

When the destroy() api's sole purpose is to delete the object on the C side, the api should be fn destroy(self) rather than pub fn destroy(&self) and be called automatically on Drop.

@i509VCB
Copy link
Member

i509VCB commented Jan 4, 2023

I think this is more appropriately addressed in the implementing libraries like sctk and smithay.

@A6GibKm
Copy link
Author

A6GibKm commented Jan 4, 2023

I mean the objects are completely useless after calling destroy, destroy should not take a reference and leave you with a useless object.

@cmeissl
Copy link
Collaborator

cmeissl commented Jan 4, 2023

At least on the server a destroyed surface for example can still be usable. Smithay puts all the surface state including the imported textures in the surface user data. This could be used for fade-out animations to give an example. Not sure if there are use-cases for that in a client.

@elinorbgr
Copy link
Member

The main difficulty with that is, if clients are the one controlling the lifetime of the objects most of the time, that is not always the case. This makes it very difficult to build a static model of the lifetime of wayland objects. I've tried quite a lot since I started wayland-rs 7 years ago, and the current fully-dynamic tracking of object lifetime is the result of me failing to do that.

So, I won't say because I failed to do it it must be impossible, but at this point I'm not willing to try again without some serious design proposal that handles well the fact that objects can be destroyed by the other end of the socket.

@ids1024
Copy link
Member

ids1024 commented Jan 16, 2024

One solution could be to destroy on drop by default, but method to set a flag on an object to disable automatic calls to the destructor. That might be more intuitive since Rust users expect a "destructor" to be called automatically through RAII, and may help make sure objects aren't accidentally leaked. Though that's more complicated in some ways.

Requiring users of the library to keep a reference to any object they don't want destroyed seems problematic though.

Otherwise, perhaps documentation can be improved. It's certainly surprising behavior, even if it ultimately makes sense.

@kchibisov
Copy link
Member

I'm against auto-destroying since it could end up in doing double free and thus breaking the order of destroy objects. In general, to handle this correctly, you'd need to ref-count and handle that ref-count across the ffi, which is complicated.

Like this really doesn't work and faces the same issues raw-window-handle has, since everything could go through ffi at any point and you should globally track that kind of stuff.

@axelkar
Copy link

axelkar commented Apr 2, 2024

Could destroy be made into a trait so I can make a "destroy on drop" wrapper easily?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants