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

backend: Fix Wayland backend idleCallbacks destruction #151

Merged
merged 2 commits into from
Mar 6, 2025

Conversation

PlasmaPower
Copy link
Contributor

The old code was crashing for me on shutdown due to the fact that CWaylandBackend's reverse field order destruction destroys idleCallbacks before destroying outputs, meaning that when ~CWaylandOutput calls backend->idleCallbacks.clear(); it's clearing an already destroyed vector which crashes.

To fix this I just removed the FIXME hack and properly handled the potential sendFrameAndSetCallback UAF by using the weak pointer to check if the output was already destroyed or not.

@vaxerski
Copy link
Member

vaxerski commented Mar 2, 2025

why not just if (backend) then?

@PlasmaPower
Copy link
Contributor Author

You mean checking that when idleCallbacks.clear() is called? I thought I might as well fix the FIXME while I was at it, though yeah checking !backend.expired() works (the expired check is necessary because this'll be called by the destructor of the backend so backend is still truthy). If you mean checking if (backend) in the callback, the concern there is that the output might be destroyed before the backend is I believe, and then the idle callback would be referencing an invalid output when called.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I re-read it and noticed that github folded the diff weirdly. Now that makes sense, yes.

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@vaxerski vaxerski merged commit 8149856 into hyprwm:main Mar 6, 2025
1 check passed
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.

2 participants