Run the BlueZ controllers on a single EventLoop, drop the mutexes#22
Merged
Conversation
Convert UdevMonitor from a self-threaded object into an EventSource and drive the D-Bus connection, the udev monitor, and signal delivery from one EventLoop per controller. Because the object-manager callbacks and the udev callback now run on the same loop thread, the per-map mutexes and the input_reader_ mutex are removed. - UdevMonitor: set up the netlink monitor in the constructor, expose fd() and drain events in dispatch(); no worker thread or self-pipe. - SignalSource: a signalfd EventSource that stops the loop on SIGINT/SIGTERM, constructed before any thread starts so the signals are blocked process-wide and delivered only through the loop's signalfd (otherwise spdlog's periodic flush thread can take the default SIGTERM disposition and kill the process). - The three controller mains replace enterEventLoopAsync() + monitorLoop() with EventLoop::run(), registering the controller (a UdevMonitor) and a SignalSource. InputReader keeps its own read thread for now (it only logs and touches no shared owner state); folding it into the loop and converting the in-callback UPower construction to async calls is the next step. Verified at runtime: the clients start, the loop drives the D-Bus callbacks, and SIGTERM is delivered through the signalfd for a clean exit. The HID input data path needs real controller hardware and is build-verified only. Signed-off-by: Joel Winarske <joel.winarske@linux.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Step 2 of the event-loop source contract, building on the
EventLoopfrom #21. Moves the three BlueZ game-controller clients onto a single-threaded event loop and deletes the concurrency that PR #18 had to harden.What changed
UdevMonitoris now anEventSource— the netlink monitor is set up in the constructor;fd()exposes the socket anddispatch()drains queued device events. Its worker thread and self-pipe are gone.SignalSource(new, insrc/utils/) — a signalfdEventSourcethat stops the loop on SIGINT/SIGTERM. It's constructed before any thread starts so the signals are blocked process-wide and delivered only through the loop.enterEventLoopAsync()+monitorLoop()withEventLoop::run(), registering the controller (itself aUdevMonitor) and aSignalSource.adapters_/devices_/input1_/upower_clients_andinput_reader_). The D-Bus object-manager callbacks and the udev callback now run on the same loop thread, so the state they share needs no locking — this retires the exact cross-thread surface behind PR Fix reliability, security, and performance issues across clients #18's race fixes. Net −122 lines.Validation
EventLoop::run()drives the D-Bus callbacks on the loop thread (object-manager/property callbacks observed firing), and SIGTERM is delivered through the signalfd —SignalSource: received signal 15, stopping→ clean exit 0. Caught and fixed a real bug along the way: with the oldmainordering, spdlog's periodic-flush thread was started before signals were blocked and took the default SIGTERM disposition, killing the process; constructingSignalSourcefirst fixes it.EventSourcemechanism theSignalSourceexercises at runtime (couldn't inject a real uevent in the CI/sandbox —udevadm triggeris unavailable there).InputReaderis unchanged here (still on its own read thread), so that path is as it was in Fix reliability, security, and performance issues across clients #18.--Werrorand clang-tidy-19 clean; full default build green.Scope / next
This increment does the structural unification (udev + D-Bus + signals on one loop). It intentionally does not yet:
InputReaderinto the loop as a source (it keeps its read thread; it only logs and touches no shared owner state), orGetAll+EnumerateDevices) to the async sdbus API.Those two are the next increment — the async conversion is the approach chosen for keeping the single loop from blocking on in-callback D-Bus round-trips. Until then, UPower client construction runs synchronously on the loop thread (a brief stall on controller connect), which is acceptable for these monitor daemons.
Stacked originally on #21 (now merged); rebased onto
main.