[GH-ISSUE #490] Joypad is unplugged from the lobby container right after being plugged in (race on JoinLobbyEvent) #298

New issue

Open

opened 2026-10-15 12:59:27 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-15 12:59:27 +00:00

Owner

Originally created by @piotrminkina on GitHub (Aug 28, 2026).
Original GitHub issue: https://github.com/games-on-whales/wolf/issues/490

Summary

When a Moonlight session joins a lobby, the joypads it already owns are moved from the session's own container into the lobby container. The move fires an UnplugDeviceEvent that is intended for the session's runner, but the lobby-side UnplugDeviceEvent handler picks it up as well and re-fires it at the lobby — deleting the device node that the accompanying plug is concurrently creating.

Because the plug goes through lobby->plugged_devices_queue (drained asynchronously) while the unplug is executed inline, which of the two wins is timing-dependent. With N pads it is N independent coin flips, which matches the field reports of "3-4 pads worked, after a reconnect only one did".

leave_lobby() performs the mirror-image operation and gets it right; JoinLobbyEvent does not. The difference is the ordering against connected_sessions.

Where (`stable`, `7f416ec` — also present in the `d6d41de` image build)

src/moonlight-server/sessions/lobbies.cpp:209 adds the session to lobby->connected_sessions first, so from that point on both lobby-side handlers match this session_id. Then, for every joypad (lobbies.cpp:220-238):

events::PlugDeviceEvent plug_ev{.session_id = std::to_string(session->session_id)};
...
app_state->event_bus->fire_event(immer::box<events::PlugDeviceEvent>(plug_ev));      // (A)
// Unplug it from the current session
app_state->event_bus->fire_event(immer::box<events::UnplugDeviceEvent>{              // (B)
    events::UnplugDeviceEvent{.session_id = std::to_string(session->session_id), ...}});

// Add it to the current lobby devices queue
lobby->plugged_devices_queue->push(immer::box<events::PlugDeviceEvent>{plug_ev});    // (C)

(A) is caught by the lobby-side PlugDeviceEvent handler (lobbies.cpp:294), which logs [LOBBY] adding device to session ... in lobby ... and pushes a copy with .session_id = lobby->id onto lobby->plugged_devices_queue. This is the path that actually works.
(B) is the bug. Its comment says "unplug it from the current session", i.e. it targets the session's own runner. But the lobby-side UnplugDeviceEvent handler (lobbies.cpp:311) has no way to distinguish "this pad is being moved into the lobby" from "the user physically unplugged this pad", so it re-fires the event with .session_id = lobby->id. The lobby's docker runner (runners/docker.cpp:225-227) matches and synchronously execs fake-udev -m <remove> && rm <DEVNAME> (docker.cpp:244) plus std::filesystem::remove(hw_db_path / filename) — in the container the pad is being moved into.
(C) pushes an event still carrying the moonlight session_id onto the lobby's queue. The runner filters with device_ev->get().session_id == session_id where session_id == lobby->id (docker.cpp:255), so this push is always discarded. It looks like a leftover duplicate of (A).

The asymmetry that turns this into a race: the plug is queued and drained by the runner loop (docker.cpp:254 pop(50ms), docker.cpp:280 sleep_for(500ms)), while the unplug is executed inline in the handler.

For comparison, leave_lobby() (lobbies.cpp:17-53) removes the session from connected_sessions before touching the joypads, then fires Plug{session_id = session} (no longer matched by the lobby handler → correctly reaches the session's own queue) and Unplug{.session_id = lobby.id} directly, which the lobby-side handler cannot re-fire because connected_sessions holds moonlight session ids, never lobby ids. No race.

Evidence

Steam lobby, 3 h session, single client. Tail of the Wolf log:

12:28:59  [LOBBY] Moonlight stream 3714165004394080558 over, leaving lobby 276857eb-...
12:28:59  [LOBBY] Session 3714165004394080558 leaving lobby 276857eb-...
12:30:40  Creating Xbox joypad for controller 0 in session 3714165004394080558
12:30:46  [LOBBY] Session 3714165004394080558 joining lobby 276857eb-...
12:30:46  [LOBBY] adding device to session 3714165004394080558 in lobby 276857eb-...

State of the lobby container immediately after, and for the following two hours:

/dev/input/       empty   (dir mtime == 12:30:46)
/run/udev/data/   empty   (dir mtime == 12:30:46)

The node was created and removed within that same moment. Note that this join produced no Docker exec failed warning, i.e. the rm returned 0, i.e. the node did exist when the unplug ran — the plug had already landed and was wiped.

The other side of the same problem is visible as noise: the log contains 43 × Docker exec failed (1), and every single one of them is

rm: cannot remove '/dev/input/js0': No such file or directory
rm: cannot remove '/dev/input/event258': No such file or directory

with zero mknod failures — removes broadcast at a container that does not hold the device.

All plug/unplug logging in docker.cpp is logs::debug, so at the default log level only the [LOBBY] lines and these Docker exec failed warnings are visible.

Reproduce

Wolf stable, one Moonlight client, an app running in a lobby, at least one controller connected to the client.
Disconnect the Moonlight stream (session leaves the lobby, pads are handed back to the Wolf UI session).
Reconnect Moonlight with the controller still connected and re-enter the same lobby.
The app no longer sees the controller. With 3-4 controllers, typically only some of them survive the rejoin — a different subset each time.

Workaround that avoids it deterministically: disconnect every controller from the client before rejoining, join the lobby, and only then connect the controllers. With session->joypads empty the loop at lobbies.cpp:221 does nothing, and each pad then arrives via create_new_joypad(), which fires only a PlugDeviceEvent with no accompanying unplug. Verified working.

Suggested fix

Mirror what leave_lobby() already does — the ordering against connected_sessions is what makes it safe:

Fire the "unplug from the origin session" before adding the session to lobby->connected_sessions, so the lobby-side handler does not match it and cannot re-fire it. (Or give the move its own event/flag so the lobby-side UnplugDeviceEvent handler skips the re-broadcast.)
Address the lobby side explicitly instead of relying on the re-fire, i.e. fix (C) to .session_id = lobby->id and drop (A), or drop (C) and keep (A). Right now (C) is dead code and (A) is load-bearing, which is almost certainly the opposite of what was intended.

Worth noting that simply deleting (B) is not sufficient: the pad would then stay plugged into the Wolf UI container, and every join/leave cycle would leave another stale set of nodes behind there — the leak just moves rather than disappearing.

I am happy to prepare a PR if you agree with the direction.

Update — this is very likely already fixed on an existing branch

fix/joypad-input-leak (single commit 920fea3, "fix(lobbies): stop gamepad input leaking into Wolf-UI across lobby switches", targeting #435) routes every join/leave hand-off through a new migrate_joypad() helper that does unplug-from-source → inputtino::Joypad::recreate_device() → plug-into-target. That ordering fixes this race by construction, and the "Suggested fix" section above should be read as superseded by it.

Worth flagging that the author almost certainly did not know they were fixing this. The branch is based on 2f790f6, 21 commits behind stable, and predates a9d7c60 (#452) — on its base the lobby UnplugDeviceEvent relay was still dead code, so this race could not occur there and the ordering was never validated against a live relay.

Two things that do not survive a rebase onto current stable:

1. A spurious unplug still reaches the lobby on every join. connected_sessions->update remains before the joypad loop, so the live relay catches migrate_joypad's step-1 Unplug{session_id} and re-broadcasts it as Unplug{lobby.id} — a synchronous docker exec into a container that does not have the node yet, one per pad per join, inside the join's 2 s deadline. It is harmless only by accident: nothing states or tests the invariant "the lobby has no node at join time". It also reproduces exactly the Docker exec failed (1) … rm: cannot remove noise that made this bug hard to diagnose in the first place.

A targeted way to make the intent explicit rather than positional — the hand-off knows it is a hand-off, so let the event say so:

// events/events.hpp
 struct UnplugDeviceEvent {
   ...
+  /** True when the device is handed over between containers (migrate_joypad) rather than actually
+   *  disconnected. The lobby relay must not re-broadcast a transfer: the runner it would target is
+   *  the one we are about to plug the device INTO. */
+  bool is_transfer = false;
 };

set it in migrate_joypad's step 1, and return early in the relay handler when it is set. UnplugDeviceEvent has no Reflector and does not appear in spec.json, so the field is purely additive.

2. Defect 3 above survives verbatim. In the join call source == target == session_id, and target_queue is lobby->plugged_devices_queue — so the direct push still carries the moonlight session id into the lobby's queue and the runner still discards it (docker.cpp:255). The commit's own comment explains that the working plug goes through the relay's re-stamp, which makes the push redundant rather than merely mis-stamped. Removing it leaves target_queue always nullptr (leave already passes that), so the parameter can go too.

Dependency the branch does not declare: 920fea3 does not bump the inputtino pin, and recreate_device() exists in neither fd136cf (the pinned commit) nor inputtino's current stable — only in the open games-on-whales/inputtino#39. As it stands the branch cannot compile against either.

Happy to prepare a PR on top of that branch with the two items above plus a regression test — there is currently no test touching migrate_joypad, and tests/testWolfAPI.cpp already has the seam for one (it builds a Lobby straight into state, no compositor, no Docker).

Originally created by @piotrminkina on GitHub (Aug 28, 2026). Original GitHub issue: https://github.com/games-on-whales/wolf/issues/490 ### Summary When a Moonlight session joins a lobby, the joypads it already owns are moved from the session's own container into the lobby container. The move fires an `UnplugDeviceEvent` that is intended for the *session's* runner, but the lobby-side `UnplugDeviceEvent` handler picks it up as well and re-fires it at the lobby — deleting the device node that the accompanying plug is concurrently creating. Because the plug goes through `lobby->plugged_devices_queue` (drained asynchronously) while the unplug is executed inline, which of the two wins is timing-dependent. With N pads it is N independent coin flips, which matches the field reports of "3-4 pads worked, after a reconnect only one did". `leave_lobby()` performs the mirror-image operation and gets it right; `JoinLobbyEvent` does not. The difference is the ordering against `connected_sessions`. ### Where (`stable`, 7f416ec — also present in the `d6d41de` image build) `src/moonlight-server/sessions/lobbies.cpp:209` adds the session to `lobby->connected_sessions` **first**, so from that point on both lobby-side handlers match this `session_id`. Then, for every joypad (`lobbies.cpp:220-238`): ```cpp events::PlugDeviceEvent plug_ev{.session_id = std::to_string(session->session_id)}; ... app_state->event_bus->fire_event(immer::box<events::PlugDeviceEvent>(plug_ev)); // (A) // Unplug it from the current session app_state->event_bus->fire_event(immer::box<events::UnplugDeviceEvent>{ // (B) events::UnplugDeviceEvent{.session_id = std::to_string(session->session_id), ...}}); // Add it to the current lobby devices queue lobby->plugged_devices_queue->push(immer::box<events::PlugDeviceEvent>{plug_ev}); // (C) ``` - **(A)** is caught by the lobby-side `PlugDeviceEvent` handler (`lobbies.cpp:294`), which logs `[LOBBY] adding device to session ... in lobby ...` and pushes a copy with `.session_id = lobby->id` onto `lobby->plugged_devices_queue`. This is the path that actually works. - **(B)** is the bug. Its comment says "unplug it from the current session", i.e. it targets the session's own runner. But the lobby-side `UnplugDeviceEvent` handler (`lobbies.cpp:311`) has no way to distinguish "this pad is being moved into the lobby" from "the user physically unplugged this pad", so it re-fires the event with `.session_id = lobby->id`. The lobby's docker runner (`runners/docker.cpp:225-227`) matches and synchronously execs `fake-udev -m <remove> && rm <DEVNAME>` (`docker.cpp:244`) plus `std::filesystem::remove(hw_db_path / filename)` — in the container the pad is being moved *into*. - **(C)** pushes an event still carrying the **moonlight** `session_id` onto the **lobby's** queue. The runner filters with `device_ev->get().session_id == session_id` where `session_id == lobby->id` (`docker.cpp:255`), so this push is always discarded. It looks like a leftover duplicate of (A). The asymmetry that turns this into a race: the plug is queued and drained by the runner loop (`docker.cpp:254` `pop(50ms)`, `docker.cpp:280` `sleep_for(500ms)`), while the unplug is executed inline in the handler. For comparison, `leave_lobby()` (`lobbies.cpp:17-53`) removes the session from `connected_sessions` **before** touching the joypads, then fires `Plug{session_id = session}` (no longer matched by the lobby handler → correctly reaches the session's own queue) and `Unplug{.session_id = lobby.id}` **directly**, which the lobby-side handler cannot re-fire because `connected_sessions` holds moonlight session ids, never lobby ids. No race. ### Evidence Steam lobby, 3 h session, single client. Tail of the Wolf log: ``` 12:28:59 [LOBBY] Moonlight stream 3714165004394080558 over, leaving lobby 276857eb-... 12:28:59 [LOBBY] Session 3714165004394080558 leaving lobby 276857eb-... 12:30:40 Creating Xbox joypad for controller 0 in session 3714165004394080558 12:30:46 [LOBBY] Session 3714165004394080558 joining lobby 276857eb-... 12:30:46 [LOBBY] adding device to session 3714165004394080558 in lobby 276857eb-... ``` State of the lobby container immediately after, and for the following two hours: ``` /dev/input/ empty (dir mtime == 12:30:46) /run/udev/data/ empty (dir mtime == 12:30:46) ``` The node was created and removed within that same moment. Note that this join produced **no** `Docker exec failed` warning, i.e. the `rm` returned 0, i.e. the node *did* exist when the unplug ran — the plug had already landed and was wiped. The other side of the same problem is visible as noise: the log contains **43 × `Docker exec failed (1)`**, and every single one of them is ``` rm: cannot remove '/dev/input/js0': No such file or directory rm: cannot remove '/dev/input/event258': No such file or directory ``` with **zero** `mknod` failures — removes broadcast at a container that does not hold the device. All plug/unplug logging in `docker.cpp` is `logs::debug`, so at the default log level only the `[LOBBY]` lines and these `Docker exec failed` warnings are visible. ### Reproduce 1. Wolf `stable`, one Moonlight client, an app running in a lobby, at least one controller connected to the client. 2. Disconnect the Moonlight stream (session leaves the lobby, pads are handed back to the Wolf UI session). 3. Reconnect Moonlight **with the controller still connected** and re-enter the same lobby. 4. The app no longer sees the controller. With 3-4 controllers, typically only some of them survive the rejoin — a different subset each time. Workaround that avoids it deterministically: disconnect **every** controller from the client before rejoining, join the lobby, and only then connect the controllers. With `session->joypads` empty the loop at `lobbies.cpp:221` does nothing, and each pad then arrives via `create_new_joypad()`, which fires only a `PlugDeviceEvent` with no accompanying unplug. Verified working. ### Suggested fix Mirror what `leave_lobby()` already does — the ordering against `connected_sessions` is what makes it safe: 1. Fire the "unplug from the origin session" **before** adding the session to `lobby->connected_sessions`, so the lobby-side handler does not match it and cannot re-fire it. (Or give the move its own event/flag so the lobby-side `UnplugDeviceEvent` handler skips the re-broadcast.) 2. Address the lobby side explicitly instead of relying on the re-fire, i.e. fix (C) to `.session_id = lobby->id` and drop (A), or drop (C) and keep (A). Right now (C) is dead code and (A) is load-bearing, which is almost certainly the opposite of what was intended. Worth noting that simply deleting (B) is *not* sufficient: the pad would then stay plugged into the Wolf UI container, and every join/leave cycle would leave another stale set of nodes behind there — the leak just moves rather than disappearing. I am happy to prepare a PR if you agree with the direction. --- ## Update — this is very likely already fixed on an existing branch `fix/joypad-input-leak` (single commit `920fea3`, "fix(lobbies): stop gamepad input leaking into Wolf-UI across lobby switches", targeting #435) routes every join/leave hand-off through a new `migrate_joypad()` helper that does unplug-from-source → `inputtino::Joypad::recreate_device()` → plug-into-target. That ordering fixes this race **by construction**, and the "Suggested fix" section above should be read as superseded by it. Worth flagging that the author almost certainly did not know they were fixing this. The branch is based on `2f790f6`, 21 commits behind `stable`, and **predates `a9d7c60` (#452)** — on its base the lobby `UnplugDeviceEvent` relay was still dead code, so this race could not occur there and the ordering was never validated against a live relay. Two things that do not survive a rebase onto current `stable`: **1. A spurious unplug still reaches the lobby on every join.** `connected_sessions->update` remains *before* the joypad loop, so the live relay catches `migrate_joypad`'s step-1 `Unplug{session_id}` and re-broadcasts it as `Unplug{lobby.id}` — a synchronous `docker exec` into a container that does not have the node yet, one per pad per join, inside the join's 2 s deadline. It is harmless only by accident: nothing states or tests the invariant "the lobby has no node at join time". It also reproduces exactly the `Docker exec failed (1) … rm: cannot remove` noise that made this bug hard to diagnose in the first place. A targeted way to make the intent explicit rather than positional — the hand-off knows it is a hand-off, so let the event say so: ```cpp // events/events.hpp struct UnplugDeviceEvent { ... + /** True when the device is handed over between containers (migrate_joypad) rather than actually + * disconnected. The lobby relay must not re-broadcast a transfer: the runner it would target is + * the one we are about to plug the device INTO. */ + bool is_transfer = false; }; ``` set it in `migrate_joypad`'s step 1, and return early in the relay handler when it is set. `UnplugDeviceEvent` has no `Reflector` and does not appear in `spec.json`, so the field is purely additive. **2. Defect 3 above survives verbatim.** In the join call `source == target == session_id`, and `target_queue` is `lobby->plugged_devices_queue` — so the direct push still carries the moonlight session id into the lobby's queue and the runner still discards it (`docker.cpp:255`). The commit's own comment explains that the working plug goes through the relay's re-stamp, which makes the push redundant rather than merely mis-stamped. Removing it leaves `target_queue` always `nullptr` (leave already passes that), so the parameter can go too. **Dependency the branch does not declare:** `920fea3` does not bump the inputtino pin, and `recreate_device()` exists in neither `fd136cf` (the pinned commit) nor inputtino's current `stable` — only in the open games-on-whales/inputtino#39. As it stands the branch cannot compile against either. Happy to prepare a PR on top of that branch with the two items above plus a regression test — there is currently no test touching `migrate_joypad`, and `tests/testWolfAPI.cpp` already has the seam for one (it builds a `Lobby` straight into state, no compositor, no Docker).