fix: prevent fork loop and JSON hang in keybinds hint - #1826
Conversation
keybinds_hint.sh: replace 'exec $0' with 'exit 0' to prevent infinite self-re-execution when rofi produces no output or dispatch fails. This was causing fork storms (~568 forks/s of python3) that led to system load spikes. hint-hyprland.py: replace infinite 'while True' retry loop with a bounded approach. When 'hyprctl binds -j' returns invalid JSON (known issue on Hyprland 0.56.0), fall back to parsing the text output of 'hyprctl binds' instead of sleeping and retrying forever. Added timeout=5 to subprocess calls to prevent indefinite hangs. Tested on Hyprland 0.56.0 + kernel 7.1.4 where 'hyprctl binds -j' produces malformed JSON (fields shifted, non-numeric values in numeric fields).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe keybind helpers now use bounded ChangesHyprland keybind handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant HintScript
participant Hyprctl
participant BindParser
HintScript->>Hyprctl: Run binds -j with five-second timeout
Hyprctl-->>HintScript: JSON output or command failure
HintScript->>BindParser: Parse plain-text binds when JSON fails
BindParser-->>HintScript: Return normalized bind dictionaries
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Configs/.local/lib/hyde/keybinds/hint-hyprland.py`:
- Around line 45-67: Update the keycode parsing branch to accept signed numeric
values such as -1 instead of relying on value.isdigit(), preserving the parsed
integer for map_codeDisplay(). In the keybind parsing logic, replace the direct
value.lower() == "true" checks for repeat, locked, mouse, release,
non_consuming, and allow_input_capture with the existing parse_bool() helper so
numeric flags such as 1 remain valid.
- Around line 31-34: Update the bind-header detection in the parser so every
Hyprland bind variant, including flagged and combined forms such as bindl=,
bindm=, bindr=, bindl:m=, and bindr:Super=, starts a new record. Preserve the
existing current-record append and reset behavior in the surrounding parsing
logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9fdcfefa-65dc-4be5-940e-5f451330b517
📒 Files selected for processing (2)
Configs/.local/lib/hyde/keybinds/hint-hyprland.pyConfigs/.local/lib/hyde/keybinds_hint.sh
| if current: | ||
| binds.append(current) | ||
| current = {} | ||
| elif ":" in line: |
There was a problem hiding this comment.
Suggestion: this fallback is parsing a schema, rather than expressing distinct control flow. A field-to-parser mapping would make new Hyprland fields a one-line addition and, with explicit defaults, ensure partially parsed binds cannot later fail with missing keys in expand_meta_data.
BIND_DEFAULTS = {
"modmask": 0,
"submap": "",
"key": "",
"keycode": 0,
"catch_all": False,
"description": "",
"has_description": False,
"dispatcher": "",
"arg": "",
"repeat": False,
"locked": False,
"mouse": False,
"release": False,
"non_consuming": False,
"allow_input_capture": False,
}
parse_bool = lambda value: value.lower() == "true"
parse_int = lambda value: int(value) if value.isdigit() else 0
FIELD_PARSERS = {
"modmask": ("modmask", parse_int),
"submap": ("submap", str),
"key": ("key", str),
"keycode": ("keycode", parse_int),
"catchall": ("catch_all", parse_bool),
"description": ("description", str),
"dispatcher": ("dispatcher", str),
"arg": ("arg", str),
"repeat": ("repeat", parse_bool),
"locked": ("locked", parse_bool),
"mouse": ("mouse", parse_bool),
"release": ("release", parse_bool),
"non_consuming": ("non_consuming", parse_bool),
"allow_input_capture": ("allow_input_capture", parse_bool),
}
# `startswith` also accommodates other Hyprland bind variants.
if line.startswith("bind"):
if current:
current["has_description"] = bool(current["description"])
binds.append(current)
current = BIND_DEFAULTS.copy()
elif ":" in line:
key, _, value = line.partition(":")
field = FIELD_PARSERS.get(key.strip())
if field:
destination, parser = field
current[destination] = parser(value.strip())- Recognize all bind header variants (bindl=, bindm=, bindr=, etc.)
not just 'bind' and 'bindd', using line.startswith('bind')
- Add parse_bool() helper to accept numeric flags (1/0) in addition
to string 'true'/'false' for repeat, locked, mouse, release,
non_consuming, allow_input_capture, and catchall fields
- Accept signed keycode values (e.g. -1) via try/except instead of
value.isdigit() which rejected negative numbers
|
Hi! I don't think adding a fix for a minor bug ( broken json) is needed. 0.56.1 fixes the json output already. Hyprland is fast changing, adding more guardrails for minor bug that was eventually fix doesn't make sense to me. Thanks! |
Problem
Two bugs in the keybinds hint system were causing system-wide performance degradation (fork storms with ~568 forks/s, load average spikes to 100+).
Bug 1:
keybinds_hint.sh— infinite self-re-execution (exec $0)The script uses
exec $0in two places as a fallback when rofi produces no output or when a dispatch fails. This causes the script to re-execute itself indefinitely, spawning hundreds of python3 processes per second.Before:
Fix: Replace
exec $0withexit 0. If the script reaches a dead end, it should exit, not loop.Bug 2:
hint-hyprland.py— infinite retry on broken JSONget_hyprctl_binds()had awhile Trueloop that retriedhyprctl binds -jforever when JSON parsing failed. On Hyprland 0.56.0,hyprctl binds -jreturns malformed JSON (fields shifted, non-numeric values in numeric fields), causing the script to hang indefinitely in the retry loop.Fix:
hyprctl binds(no-jflag)timeout=5to prevent indefinite hangsImpact
Super+/) now works on Hyprland 0.56.0 wherehyprctl binds -jis brokenTesting
Tested on:
Summary by CodeRabbit