Fix race condition in start-dumb-udev.sh causing repeated Xorg restarts - #239
Open
qdii wants to merge 2 commits into
Open
Fix race condition in start-dumb-udev.sh causing repeated Xorg restarts#239qdii wants to merge 2 commits into
qdii wants to merge 2 commits into
Conversation
start-dumb-udev.sh restarts Xorg once when it notices a Sunshine virtual input device (or an evdev passthrough device) appear, so Xorg can pick it up. It remembers having done this with a flag file, and only allows itself to restart Xorg again once that device has gone away. The problem is a race between two things happening on their own timelines: the watcher checks for the device once a second, while Sunshine removes and recreates its virtual devices independently (for example while a client keeps retrying a connection). If the watcher's check happens to land in the brief moment between Sunshine removing the old device and creating the new one, it thinks the device is gone for good and resets its flag. The moment the new device appears a second later, it looks like a brand new device to the watcher, which restarts Xorg again. A flaky client connection can turn this into a continuous Xorg restart loop, taking down the whole desktop session. The fix: don't trust a single absent check. The watcher now needs to see the device missing for several consecutive seconds before it treats it as actually gone (configurable via ABSENCE_DEBOUNCE_SECONDS, default 10). A quick remove-then-recreate no longer falls inside that window, so it's no longer mistaken for a fresh device needing a fresh restart.
The logic was inverted in the if condition
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.
start-dumb-udev.sh restarts Xorg once when it notices a Sunshine virtual input device (or an evdev passthrough device) appear, so Xorg can pick it up. It remembers having done this with a flag file, and only allows itself to restart Xorg again once that device has gone away.
The problem is a race between two things happening on their own timelines: the watcher checks for the device once a second, while Sunshine removes and recreates its virtual devices independently (for example while a client keeps retrying a connection). If the watcher's check happens to land in the brief moment between Sunshine removing the old device and creating the new one, it thinks the device is gone for good and resets its flag. The moment the new device appears a second later, it looks like a brand new device to the watcher, which restarts Xorg again. A flaky client connection can turn this into a continuous Xorg restart loop, taking down the whole desktop session.
The fix: don't trust a single absent check. The watcher now needs to see the device missing for several consecutive seconds before it treats it as actually gone (configurable via ABSENCE_DEBOUNCE_SECONDS, default 10). A quick remove-then-recreate no longer falls inside that window, so it's no longer mistaken for a fresh device needing a fresh restart.