hs.window.switcher memory leak(s)

[joshkaplan]

Summary: When creating a hs.window.switcher, it never releases some memory. If creating and using multiple switchers dynamically (i.e. for each application upon focus), Hammerspoon can quickly reach multiple GB of memory simply by using the switchers.

Details: I messed with the hs.window.switcher code a bit, but could not fully fix. I believe there are multiple root issues:

1. When creating multiple hs.window.switcher (i.e. for the current active application), they are never garbage collected (the __gc method does not fire until Hammerspoon is restarted.) This appears to be an issue with the stored hs.screen.watcher. If I manually set this to nil, garbage collection of the switcher does occur.
2. However, even with garbage collection of the switcher taking place, memory still builds up. I suspect this has to do with with the hs.drawing objects. I attempted to manually :delete() each of these when GC'ing, but that did not solve the issue either 😢 . It's possible I am simply failing to delete all the drawings... that part of the code was a bit hard to follow for me.

To recreate:

1. Set up any hook that creates multiple application switchers. This is the code I use:

   
   function get_current_app_switcher_vis()
   local app = hs.application.frontmostApplication()
   local wf = hs.window.filter.new(false):setAppFilter(app:name(), {visible = true, currentSpace = true}):setSortOrder(hs.window.filter.sortByFocusedLast)
   local switcher = hs.window.switcher.new(wf, {}, 'switcher', 'debug') -- add logging
   return switcher
   end

-- global switcher switcher_app_vis = get_current_app_switcher_vis() bind('cmd', '', nil, function() switcher_app_vis:next() end) bind('cmd-shift', '', nil, function() switcher_app_vis:previous() end)

-- global app watcher, updates the global switcher to a new instance when app changes active_app_watcher = hs.application.watcher.new(function(name, e, app) if e == hs.application.watcher.activated then switcher_app_vis = get_current_app_switcher_vis() end end):start()

2. Activate the switcher using the hotkey (in my code, cmd + \`)
3. Create a new switcher (in my code, simply by switching the active apps)
4. Repeat steps 2-3 and watch the memory skyrocket

__Final Note__: It doesn't look like this module has been touched in a few years, and it's based on the now-deprecated `hs.drawing`. But figured I'd share my findings for knowledge-sharing, in case anyone is working on it or decides to revive it at some later point. If people can point me in the right direction, perhaps I can submit a PR myself, although I'm a novice with Lua and C. Being able to see the window previews was a super useful feature!

[asmagill]

You're correct that this hasn't been touched in a while, so it may be updatable in a number of ways that would be of benefit. And I'm not sure who the original author was, so unless they are still active in the community and see this, I can't ping them directly for input.

I've put up a question (#2730) about going ahead and making canvas auto-collect that should help at least some with this issue, but its going to be a bit before I can make the time to address it, assuming no objections arise -- it would be a "breaking" change from the previous expected behavior of the canvas/drawing module.

Glancing at your code above, when switcher_app_vis gets set to a new value, the previous switcher should be collected (maybe not immediately, but soon thereafter) and if it isn't, it suggests that there may be something within the module code itself which is also capturing a reference to the switcher object so it can't be collected... it will probably be a couple of days before I can look closely at this, so feel free to look into it yourself if you've the time or inclination -- we welcome new contributors!

[cmsj]

I believe switcher was mainly written by @lowne but he's not been active in Hammerspoon for some time, so that module basically hasn't changed in 5 years.

[latenitefilms]

Worth noting that just making something nil won't automatically trigger garbage collection. If you want to force garbage collection to occur you can use collectgarbage(); collectgarbage().

[asmagill]

True. Collection can only occur when the lua interpreter is running code -- not when Hammerspoon is idle.

I didn't mention it in my above comments because with an application watcher, there is lua activity occurring every time the active application changes, so collection will (at some point) occur.

@latenitefilms's suggestion is worth noting if you have a particularly simple setup (few/no timers, few/no watchers that get triggered by things you do relatively often on the computer, etc.) or if you've done something that you know has used a lot of memory and are done with it now and want to clear it quickly rather than wait for the interpreter to get around to it.

Most users will find that they don't need to trigger collection manually, but there are always exceptions.

[asmagill]

And I should also note that if there is still a reference to the object (switcher) then forcing garbage collection won't change anything -- it still won't be collected.

The observation that memory only goes up, notably fast, and never clears until reload/restart suggests that such an "extra" reference of either the switcher itself or of the images or canvases it uses is occurring.

[joshkaplan]

we welcome new contributors!

Awesome, I'd be happy to contribute if I figure anything out that is worthwhile and sufficiently non-hacky. Huge fan of Hammerspoon, you all do great work.

when switcher_app_vis gets set to a new value, the previous switcher should be collected (maybe not immediately, but soon thereafter) and if it isn't, it suggests that there may be something within the module code itself which is also capturing a reference to the switcher object so it can't be collected

@asmagill this^ part I believe I did roughly figure out, (see first bullet in "details" in OP). Basically, I added a new method :delete() to switcher, which would do this:

self.screenWatcher:stop()
self.screenWatcher = nil

After calling :delete(), the unreferenced switchers could be successfully garbage collected 🎉 . Presumably, there was a reference within screenWatcher (it's written in C, and I don't really understand how memory management works with the C API in Lua scripts.) However, the memory leak persisted even with this fix, so this appears to only part of the problem (see second bullet in "details" in OP.)