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

Fix focus change deadlock in linux_wayland backend #508

Merged
merged 2 commits into from
Jan 2, 2025

Conversation

Adjective-Object
Copy link
Contributor

@Adjective-Object Adjective-Object commented Jan 2, 2025

Currently, if a game using the linux_wayland backend loses focus, it will deadlock with the following stack:

(gdb) bt
#0  syscall ()
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x00005583ee357846 in std::sys::pal::unix::futex::futex_wait () at std/src/sys/pal/unix/futex.rs:72
... (omitted for brevity)
#5  0x00005583ee2e0bf8 in miniquad::window::blocking_event_loop () at src/lib.rs:159
#6  0x00005583ee1e06b8 in macroquad::{impl#3}::resize_event (self=0x1, width=800, height=600)
    at src/lib.rs:506
#7  0x00005583ee2e341b in miniquad::native::linux_wayland::xdg_toplevel_handle_configure (
    data=0x7ffc4e5c5fb0, _toplevel=0x558428e38e40, width=800, height=600, _states=0x5584298b6770)
    at src/native/linux_wayland.rs:508
...
#15 0x00005583ee135d9d in miniquad::start<macroquad::{impl#5}::from_config::{closure_env#0}<miniquad::conf::Conf, cool_game::main::{async_block_env#0}>> (conf=..., f=...)
    at /workspaces/miniquad/src/lib.rs:412
...

What's happening here is that the wayland backend is holding onto the mutex guard for the native_display() when it dispatches the resize event to event listeners.

This hits the EventHandler impl for Stage in macroquad 0.4.13, which attempts to re-lock the native_display() while it is already locked by the wayland backend
https://docs.rs/crate/macroquad/0.4.13/source/src/lib.rs#506

The fix here is to guarantee that the native display is released before dispatching events.
This looks like it was already done for the payload.decorations codepath, but not for the event handlers. I can't see a motivation for continuing to hold the lock past where we read the last data off the display, so I've simply moved the manual drop up to always execute.

Currently, if a game using the linux_wayland backend loses focus, it
will deadlock with the following stack:

```
(gdb) bt
    at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
... (omitted for brevity)
    at src/lib.rs:506
    data=0x7ffc4e5c5fb0, _toplevel=0x558428e38e40, width=800, height=600, _states=0x5584298b6770)
    at src/native/linux_wayland.rs:508
...
   from /lib/x86_64-linux-gnu/libwayland-client.so.0
...
```

What's happening here is that the wayland backend is holding onto the
mutex guard for the `native_display()` when it dispatches the resize
event to event listeners.

This hits the `EventHandler` impl for `Stage` in macroquad 0.4.13, which
attempts to re-lock the `native_display()` while it is already locked by
the wayland backend
https://docs.rs/crate/macroquad/0.4.13/source/src/lib.rs#506

The fix here is to guarantee that the native display is released before
dispatching events.
This looks like it was already done for the `payload.decorations`
codepath, but not for the event handlers. I can't see a motivation for
continuing to hold the lock past where we read the last data off the
display, so I've simply moved the manual drop up to always execute.
@not-fl3 not-fl3 merged commit 709fe99 into not-fl3:master Jan 2, 2025
10 checks passed
@not-fl3
Copy link
Owner

not-fl3 commented Jan 2, 2025

Thanks for PR!

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