Skip to content
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

Memory leak with idle checking #50

Open
jacwright opened this issue Feb 16, 2019 · 4 comments
Open

Memory leak with idle checking #50

jacwright opened this issue Feb 16, 2019 · 4 comments

Comments

@jacwright
Copy link

When idle checking is on, every mouse move, keystroke, and scroll event calls wake which iterates through an array of old timers and clears them (though only effectively clearing the last timer which would be the only one ever active). This array grows unbounded with each event. An array should not be used here.

Current:

trackIdleStatus = ->
    timer = []
    wakeUp = ->
      timer.map(clearTimeout);
      ifvisible.wakeup()  if status isnt "active"
      idleStartedTime = +(new Date())
      timer.push setTimeout(->
        ifvisible.idle()  if status is "active"
      , idleTime)
...

Fix:

trackIdleStatus = ->
    timer = null # No array, just a reference to the last timeout id
    wakeUp = ->
      clearTimeout(timer); # Clear previous timeout. A noop if the timeout already ran
      ifvisible.wakeup()  if status isnt "active"
      idleStartedTime = +(new Date())
      timer = setTimeout(->
        ifvisible.idle()  if status is "active"
      , idleTime)
...
@AbelMakihara
Copy link

AbelMakihara commented Feb 22, 2019

When page is idle and mousemove to 'wake up', ifvisible.wakeUp(wakeUp) and 'mousemove' clear the same timer and set 2 timer.
So one of the timers won't be cleared and it will always trigger 'idle' callback.
That's what i found in v1.0.6. :(

@jacwright
Copy link
Author

@AbelMakihara good catch. Here is an updated fix:

Fix:

trackIdleStatus = ->
    timer = null # No array, just a reference to the last timeout id
    wakeUp = ->
      return ifvisible.wakeup()  if status isnt "active"
      clearTimeout(timer); # Clear previous timeout. A noop if the timeout already ran
      idleStartedTime = +(new Date())
      timer = setTimeout(->
        ifvisible.idle()  if status is "active"
      , idleTime)
...

@woowalker
Copy link

is still on maintain ?

@rosskevin
Copy link

I do believe this to be fixed in my fork https://github.com/rosskevin/ifvisible. If not please PR a test (expanded tests are present there)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants