fix(custom): prevent UI starvation (missing tooltips) when restart-interval is 0#4843
fix(custom): prevent UI starvation (missing tooltips) when restart-interval is 0#4843tteeaa wants to merge 1 commit intoAlexays:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a Waybar UI responsiveness issue where a custom module configured with a very small restart-interval can create a tight restart loop that starves the UI event loop, causing tooltips across the bar to stop working.
Changes:
- Adds a lower-bound clamp for
restart-intervalinCustom::continuousWorker(). - Logs a warning when the configured
restart-intervalis below the enforced minimum.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (interval_ms < 1000) { | ||
| spdlog::warn("Custom module: restart-interval ({}ms) is too low. Enforcing 1000ms to prevent UI starvation.", interval_ms); | ||
| interval_ms = 1000; | ||
| } |
There was a problem hiding this comment.
The warning will be emitted on every restart when the interval is clamped (potentially once per second indefinitely), which can spam logs for a misconfigured module. Consider logging this warning only once per module instance (e.g., track a boolean) or rate-limit it.
| if (interval_ms < 1000) { | ||
| spdlog::warn("Custom module: restart-interval ({}ms) is too low. Enforcing 1000ms to prevent UI starvation.", interval_ms); | ||
| interval_ms = 1000; | ||
| } |
There was a problem hiding this comment.
Indentation inside this if block is inconsistent with the surrounding 2-space indentation in this file; please reformat to match the existing style.
| long interval_ms = static_cast<long>(config_["restart-interval"].asDouble() * 1000); | ||
|
|
||
| // Minimum interval (1 second) to prevent UI starvation from tight loops | ||
| if (interval_ms < 1000) { | ||
| spdlog::warn("Custom module: restart-interval ({}ms) is too low. Enforcing 1000ms to prevent UI starvation.", interval_ms); | ||
| interval_ms = 1000; | ||
| } | ||
| thread_.sleep_for(std::chrono::milliseconds(interval_ms)); |
There was a problem hiding this comment.
This enforces a 1000ms minimum restart-interval, but the documented minimum for restart-interval is 1ms. This is a behavioral change that will override legitimate configurations that intentionally use sub-second restarts (e.g. 0.5s). Consider keeping the documented minimum (1ms) and instead adding a throttling/backoff mechanism only when the script exits too quickly (to prevent tight restart loops), or update the manpage/docs to match the new enforced minimum.
1 participant
Closes #4842
Bug
When a custom module script exits immediately and
"restart-interval": 0is set, tooltips for the entire bar stop working.Cause
As documented in man/waybar-custom.5.scd, the minimum restart interval is 1ms (0.001).
"Minimum value is 0.001 (1ms). Values smaller than 1ms will be set to 1ms."
A 1ms restart loop on a script that exits instantly creates a tight loop. This prevents mouse hover events (tooltips) from processing.
Fix
Fix: I have added a safety check in src/modules/custom.cpp. If the user sets a restart-interval lower than 1000ms (1 second), the module now:
This breaks the tight loop, allowing the UI to remain responsive even if a user misconfigures a fast-exiting script.
Verification
[warning] Custom module: restart-interval (0ms) too low. Enforcing 1000ms to prevent UI starvation.