closes config and file watcher when stop is called. #1798
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a resource leak issue by properly closing file watchers when the application stops. The changes ensure that both the configuration watcher and text file watchers are properly cleaned up to prevent potential hanging issues, particularly on Raspberry Pi systems.
- Adds proper cleanup for the config watcher in the main application's Stop function
- Adds proper cleanup for file watchers in the textfile widget when it receives a quit signal
- Stores watcher references as instance variables to enable proper resource management
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/wtf_app.go | Stores config watcher as instance variable and closes it in Stop function |
| modules/textfile/widget.go | Stores file watcher as instance variable and closes it on quit signal |
| @@ -128,7 +129,8 @@ func (widget *Widget) plainText() string { | |||
| } | |||
|
|
|||
| func (widget *Widget) watchForFileChanges() { | |||
| watch := watcher.New() | |||
| widget.fileWatcher = watcher.New() | |||
| watch := widget.fileWatcher | |||
There was a problem hiding this comment.
[nitpick] The local variable 'watch' is unnecessary since you can use 'widget.fileWatcher' directly throughout the function. This reduces code complexity and eliminates the need for the alias.
Copilot uses AI. Check for mistakes.
| @@ -203,7 +207,8 @@ func (wtfApp *WtfApp) scheduleWidgets() { | |||
| } | |||
|
|
|||
| func (wtfApp *WtfApp) watchForConfigChanges() { | |||
| watch := watcher.New() | |||
| wtfApp.configWatcher = watcher.New() | |||
| watch := wtfApp.configWatcher | |||
There was a problem hiding this comment.
[nitpick] The local variable 'watch' is unnecessary since you can use 'wtfApp.configWatcher' directly throughout the function. This reduces code complexity and eliminates the need for the alias.
Copilot uses AI. Check for mistakes.
|
@jonhadfield Question, did you request the Copilot review or did this PR just do that by itself? |
|
It did it by itself. I do subscribe to Copilot, so maybe it triggers on all PRs automatically. |
The watchers created in both watchForConfigChanges() (wtf-app.go) and watchForFileChanges() (modules/textfile/widget.go) aren't closed when the application is told to stop.
This PR adds the config watcher to the wtfApp instance so it can be closed in the Stop function after the widgets are closed. It does the same thing for the textfile widget so it can close the filewatcher when told to quit.
Not sure if this fixes the raspberry pi issue (hangs after reload) as I'm unable to find the historic PR, but I suspect the leftover watcher may at least contribute to the issue.