Fix #488: re-arm plugin refresh timer after cancellation#489
Open
melonamin wants to merge 2 commits into
Open
Conversation
Introduce a configurable execution timeout (default 30s) that terminates long-running plugin scripts and their child process trees. Enhance logging with execution duration, stderr capture, content previews, and visibility change tracking. Read stdout/stderr concurrently to prevent pipe deadlock.
refresh() disables the timer before scheduling a new RunPluginOperation and relies on that operation calling enableTimer() at the end of main() to re-arm it. Two paths could leave the plugin permanently dormant: - pluginInvokeQueue.cancelAllOperations() can mark the operation cancelled before main() runs, so the existing post-invoke enableTimer() call is skipped. - A cancellation observed between invoke() and the next guard makes main() return early without re-arming either. Move the enableTimer() call into a defer at the top of main() so every exit path re-arms, and call enableTimer() in refresh() itself before enqueueing the operation so the timer survives even if the operation is removed from the queue before main() executes. The cancellable.isEmpty guard inside enableTimer() prevents duplicate subscriptions. Add a regression test that cancels the operation before calling main() and verifies the timer is still armed.
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
1 participant
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.
Summary
Closes #488. The reported "memory leak when displaying images" turned out to not be a leak at all — a 4-hour run of the user's
homeweather.2m.pyplugin on macOS 26.5 with a freshly built SwiftBar showed flat fd count (42-44) and RSS slightly decreasing (71 → 60 MB). What actually happens is that the plugin's refresh timer dies and the plugin stops being executed entirely; the stale dropdown image and error icon are downstream consequences. Reproduced exactly: zero plugin child processes in any 130-second window at end of run, andswiftbar://refreshallplugins+swiftbar://refreshplugin?name=...did not revive it. Only an app restart helped — matching the user's report verbatim.What breaks the timer
refresh()disables the timer before scheduling a newRunPluginOperationand relies on that operation callingenableTimer()at the end ofmain()to re-arm it. Two paths can leave the plugin permanently dormant:pluginInvokeQueue.cancelAllOperations()(called byrefreshAllPlugins/startAllPlugins/terminateAllPlugins) can mark the operation cancelled beforemain()runs, so the existing post-invokeenableTimer()call is skipped.invoke()and the nextguard !isCancelledmakesmain()return early without re-arming either.The fix
enableTimer()into adeferat the top ofRunPluginOperation.main()so every exit path re-arms.enableTimer()inrefresh()itself before enqueueing the operation so the timer survives even if the operation is removed from the queue beforemain()executes. Thecancellable.isEmptyguard insideenableTimer()prevents duplicate subscriptions.Test plan
testRunPluginOperation_rearmsTimersForTimerArmingPluginsstill passestestRunPluginOperation_rearmsTimerWhenCancelledBeforeMainverifies the new guaranteehomeweather.2m.pyscenario should now stay alive past the previous failure window