New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eventtap randomly stops intercepting events #1103

Closed
eoranged opened this Issue Nov 28, 2016 · 9 comments

Comments

Projects
None yet
5 participants
@eoranged
Copy link

eoranged commented Nov 28, 2016

From #689 :
@asmagill

if Hammerspoon was slow for any reason, keyboard entry dropped completely system wide until Hammerspoon could respond.

I'm currently playing with eventtap (Hammerspoon 0.9.49 and 0.9.50, MacOS 10.12.1, MBP Retina, 13-inch, Early 2015) and It seems to be working fine for me, but It randomly stops to intercept events. It might be related to the issue cited above.

Currently my config looks this way: https://gist.github.com/eoranged/b05481d2a2646a84a15da88a53304d92

It usually takes ~30-90 minutes to happen, but sometimes everything works fine for couple days. I don't see any pattern here. Callbacks are not executed until I reload config.

@ddiachkov

This comment has been minimized.

Copy link

ddiachkov commented Dec 3, 2016

I have similar problem. My theory is that eventaps are collected by GC.
Try to store your taps in global variables: tap = hs.eventtap.new ... instead of local tap = ...

@cmsj

This comment has been minimized.

Copy link
Member

cmsj commented Dec 3, 2016

As a general recommendation, I would try to avoid having anything in your init.lua be a local - the init.lua's scope ends at when it's been executed, so anything marked local is a viable candidate for Garbage Collection.

(If you've seen any official docs/guides that use locals, please shout at me, and I'll fix them!)

While we're on the subject, @asmagill and @Habbie and I have had some on/off discussions about whether we should implicitly retain references to all of our created objects, so they don't get GC'd, so we may be able to avoid this horrible discovery in future, but it needs a good deal of thought since we'd then be potentially piling up objects for days/weeks/months.

@ddiachkov

This comment has been minimized.

Copy link

ddiachkov commented Dec 3, 2016

For example: http://www.hammerspoon.org/go/ section "Fancy configuration reloading"

If you put hs.timer.doAfter(5, collectgarbage) at the bottom of init.lua then config reloading will stop working in 5 sec.

The same for "Reacting to application events" and I think every other watcher.

@latenitefilms

This comment has been minimized.

Copy link
Contributor

latenitefilms commented Dec 4, 2016

As a general recommendation, I would try to avoid having anything in your init.lua be a local - the init.lua's scope ends at when it's been executed, so anything marked local is a viable candidate for Garbage Collection.

Opps! I didn't realise this.

I wrongly interpreted "A quick aside about variable lifecycles" in Getting Started with Hammerspoon to say as long as you "capture them in a variable" in init.lua (whether that be local or global), then they WON'T be "be silently destroyed at some point in the future". My bad!

So basically, what you're saying @cmsj is that the ONLY time you should use local variables is INSIDE a function, where that variable is no longer needed when the function is complete. Anything inside init.lua should ALWAYS be a global variable - otherwise, it will be subject to garbage collection, and removed at "some point" in the future.

@eoranged

This comment has been minimized.

Copy link

eoranged commented Dec 5, 2016

Thanks, Denis!
Got rid of local variables and now everything works good.

@eoranged eoranged closed this Dec 5, 2016

billputer added a commit to billputer/dotfiles that referenced this issue Dec 5, 2016

Bind eventtap and pathwatcher to global variables to avoid garbage
collection

This was causing eventtap bindings to fail after a random time because
they were being garbage collected.
Hammerspoon/hammerspoon#1103 (comment)
@latenitefilms

This comment has been minimized.

Copy link
Contributor

latenitefilms commented Dec 6, 2016

@cmsj & @asmagill - Quick question regarding the whole global vs local discussion. If a module is made global within init.lua, does that mean any local variables WITHIN this now global module will avoid garbage collection? I'm just trying to work out whether or not ALL watchers, timers, etc. need to be global or not? Thoughts? Thanks in advance!

@asmagill

This comment has been minimized.

Copy link
Member

asmagill commented Dec 6, 2016

As a general rule of thumb, all watchers and timers should be captured in a global variable.

An exception to this requirement is when a local variable is captured within the scope of a function which is captured in a global variable or the Lua registry.

If the last line doesn't make sense, see below, or take the simpler approach of just making sure to capture them in global variables.

To keep from name collisions, I separate most of my stuff into separate files, then create a table that all of the watchers, etc. get added to for that file, then return that table at the end of the file. When I invoke the file with require, I then save the returned table into _asm which is (almost) the sole global variable I add to the global name space. Since each file has a separate table in _asm, they're global, but I don't have to worry about remembering what variable names I might have used in other files since they each have their own separate returned table.

Optional Explanation of Exception listed above:

Take for example the following code (copy and paste both lines into the console as one, if you want to try it yourself):

local a = 0
increment = function() a = a + 1 ; return a end

In this case, a is local, but increment is global. Since increment directly refers to a, it has become an up-value (Google if you want more details) and won't be collected, even though the file or console entry has gone out of scope.

> a, increment()
nil	1
> a, increment()
nil	2
> a, increment()
nil	3
> a, increment()
nil	4

etc.

Similarly when you register a callback function for most of our watchers, etc. the function itself has a reference stored in a special table referred to as the LUA_REGISTRY. The registry is effectively a "global" table available to all C-API functions/methods (or with debug.getregistry() if you want to examine it yourself)

So

s = false -- global
local a = 0
local t
t = hs.timer.doEvery(1, function() if not s then a = a + 1 ; print(a) else t:stop() end end)

Gives the following output:

1
2
3
> collectgarbage() -- if we hadn't captured `t` in the callback function, this would stop the timer
0
4
5
> s, a, t
false	nil	nil
6
7
> s = true -- this finally stops the timer, since `s` was the only global
@latenitefilms

This comment has been minimized.

Copy link
Contributor

latenitefilms commented Dec 6, 2016

Legend, thanks so much @asmagill !

This needs to go in the FAQ or Getting Started with Hammerspoon - very helpful!

@asmagill

This comment has been minimized.

Copy link
Member

asmagill commented Dec 6, 2016

It's a topic that I've been planning to add to the wiki documents that I hope to work on this month.

For completeness (when I refer back to this for the wiki doc), the final example really should be more like this:

s = false -- global
local a = 0
local t
t = hs.timer.doEvery(1, function()
    if not s then
        a = a + 1
        print(a)
    else
        t:stop()
        t = nil -- without this, the timer can never actually get collected, even though we've stopped it
     end
end)

For a timer, the object is relatively small, so it would take a lot of them to actually cause a noticeable memory leak, but some of the other objects behind the modules are notably larger

shepabashi added a commit to shepabashi/dotfiles that referenced this issue Jan 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment