Ensure joy_connection_changed is emitted on the main thread
#80432
Conversation
| @@ -475,7 +475,7 @@ void Input::joy_connection_changed(int p_idx, bool p_connected, String p_name, S | |||
| } | |||
| joy_names[p_idx] = js; | |||
|
|
|||
| emit_signal(SNAME("joy_connection_changed"), p_idx, p_connected); | |||
| call_deferred("emit_signal", SNAME("joy_connection_changed"), p_idx, p_connected); | |||
There was a problem hiding this comment.
I'd add a comment to clarify, like "Defer to emit on the main thread." (IIUC).
There was a problem hiding this comment.
BTW I'm not good with multithreading, but would deferring the call to joy_connection_changed in the Linux input code solve the issue similarly?
If only some platforms do this in a thread, maybe they should be responsible for deferring to the main thread where needed?
Though this one claims to be thread-safe so I guess this helps provide this guarantee?
There was a problem hiding this comment.
Yeah, from what I've checked it seems only Linux runs on a thread, and I did try making this change there initially. But for some reason, it didn't work: on Godot 4, the signal stopped being emitted altogether; on Godot 3, the signal stopped being emitted when a joypad is disconnected, and every time I reconnected the joypad it registered a new device index.
akien-mga
added
bug
platform:linuxbsd
topic:input
cherrypick:3.x
rsubtil
force-pushed
the
fix_linux_joypad_on_thread
branch
from
125f788
to
1bb73b0
Compare
|
Thanks! |
|
Cherry-picked for 3.6. |
akien-mga
removed
the
cherrypick:3.x
|
Cherry-picked for 3.5.3. |
akien-mga
removed
the
cherrypick:3.5
YuriSizov
removed
the
cherrypick:4.1
|
Cherry-picked for 4.1.2. |
Closes #78531 (once cherry-picked for
3.x)On Linux, the code for querying joypad status runs in a thread (
JoypadLinux::joypad_events_thread_run). Therefore, thejoy_connection_changedsignal is not emitted on the main thread, which depending on user code, can lead do a deadlock due to thread-unsafe API.