Skip to content

Fix leaks of SeatRc and KbdRc, but using Weak in user data #1663

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Feb 25, 2025

This fixes #1422.

This can be tested by adding Drop impls that print to those two types, or using tools that detect leaks.

It's probably worth examining more to see why this created a cycle or otherwise didn't get freed, but if we don't anticipate issues, maybe it's best to use Weak in every possible place. I also need to look into some other reported leaks.

ids1024 added 2 commits July 29, 2025 21:37
This fixes Smithay#1422.

This can be tested by adding `Drop` impls that print to those two types,
or using tools that detect leaks.
@ids1024 ids1024 marked this pull request as ready for review July 30, 2025 16:17
@ids1024
Copy link
Member Author

ids1024 commented Jul 30, 2025

Updated now that calloop is released. LeakSanitizer in Anvil shows fewer leaks now.

@Drakulix
Copy link
Member

Any particular reason why this only addresses the KeyboardHandle and not PointerHandle and TouchHandle as well?

@ids1024
Copy link
Member Author

ids1024 commented Jul 30, 2025

Good point.

Actually, that reference cycle may have been fixed a different way by #1516 (I may do a bit more testing).

MemorySanitizer does show the calloop change addresses some leaks appearing there.

@ids1024
Copy link
Member Author

ids1024 commented Jul 31, 2025

Ah, I see. So #1516 changes known_kbds and the like to use wayland_server::Weak, while the change here uses a std::sync::Weak in KeyboardUserData and SeatUserData. Either prevents a leak, and both also works (using weak references for both doesn't cause early drops of anything since Wayland objects aren't destroyed on drop).

May be best to still make this change, as well as a similar change for pointer and touch. Avoiding strong references like this in user datas seems good in general.

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

Successfully merging this pull request may close these issues.

KbdRc and SeatRc are leaked
2 participants