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

When removing output global, use disable_global, remove with timer #1082

Merged
merged 1 commit into from
Dec 18, 2024

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Dec 18, 2024

This should fix an issue where output hotplug can sometimes cause clients (including XWayland) to crash with a protocol error trying to bind the output.

Using a timer doesn't seem ideal, but seems to be the correct way to do this at present. Wlroots wlr_global_destroy_safe is basically the same as this.

Adding a LoopHandle argument to OutputConfigurationState::new seems awkward, but maybe better than a handler method for removing globals. (IdleNotifierState::new also takes a LoopHandle). Perhaps Smithay could provide some kind of helper for this.

This should fix an issue where output hotplug can sometimes cause
clients (including XWayland) to crash with a protocol error trying to
bind the output.

Using a timer doesn't seem ideal, but seems to be the correct way to do
this at present. Wlroots `wlr_global_destroy_safe` is basically the same
as this.

Adding a `LoopHandle` argument to `OutputConfigurationState::new` seems
awkward, but maybe better than a handler method for removing globals.
(`IdleNotifierState::new` also takes a `LoopHandle`). Perhaps Smithay
could provide some kind of helper for this.
@ids1024 ids1024 requested a review from a team December 18, 2024 21:48
Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Looks good

@Drakulix Drakulix merged commit 1118aa2 into master Dec 18, 2024
7 checks passed
@Drakulix Drakulix deleted the global-remove-timer branch December 18, 2024 23:17
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