Assumed current-buffer and leaking timers

[oscarfv]

On a long-lived (24 days) Emacs session I was experiencing severe sluggishness and upon investigation M-x list-timers showed hundreds of timers created by a-g-m. Since this is an old problem that supposedly was fixed several times, now it is my turn :-)

a-g-m assumes that the current-buffer remains the same on the cycle change -> timer expiration -> indentation -> timer cancelation. The key point here is that we are using an idle timer stored on a buffer local variable. That means that no matter the delay we assign to the timer, it only will fire after Emacs is idle, and then (current-buffer) might have changed.

For instance:

$ emacs -Q foo.el
M-x load-library aggressive-indent.el
M-x aggressive-indent-mode
<type some text:>
(foo a b)
<place point between `a' and `b'>
M-: (progn (insert "\n") (switch-to-next-buffer))
M-x list-timers
<accept the warning>
<notice the timer created by a-g-m>
<if we go back to foo.el the timer will be canceled but...>
C-x s
M-x kill-buffer foo.el
<now the timer is there forever>

But how this happens on real practice? Do some automated buffer change process and save and kill the buffer without visiting it. That's my case, simplified: say that I want to change foo to bar on all source files; for that I use some text-searching tool, such as grep, and then wgrep.el that makes the grep buffer editable and allows to M-x replace-string foo bar and then wgrep applies the changes to the real files; finally I kill the buffers without visiting them. Now I have a dangling timer for each file that was modified.

The solution involves not assuming that the current buffer remains the same along the a-g-m actuation cycle mentioned above. We can store (current-buffer) in the list of changes, make the list non-local (and the timer too) and then switch to the corresponding buffer to apply each change.

I plan to do some experimentation with this approach as time allows but if there are comments (or patches! :-) about this in the meantime that suggests a better approach, the better.

[leuven65]

1. The problem can be easily reproduced by run "org-babel-tangle-file" to tangle some elisp code.
   During doing ob-tangle, it seems that it will create a lot of temporary buffers for elisp code blocks, and these buffers will be killed finally.
   However the timer function "aggressive-indent--indent-if-changed" can't handle this case correctly.

2. The root cause is that the variable "aggressive-indent--idle-timer" is buffer local, it means the timer handle will be lost after the buffer is killed, and "aggressive-indent" can't cancel this timer anymore.

   I made a bugfixing on https://github.com/leuven65/aggressive-indent-mode/commit/f611f3114b55e92a03e978cd0721b21e58523f48

   The fixing is simple:

   (defun aggressive-indent--on-buffer-kill ()
     "Cancel the timer before buffer is killed"
     (when (timerp aggressive-indent--idle-timer)
       (cancel-timer aggressive-indent--idle-timer)
       (setq aggressive-indent--idle-timer nil)))
   (add-hook 'kill-buffer-hook #'aggressive-indent--on-buffer-kill nil t)

[tmurph]

Bump? I noticed the same issue (thousands of dangling timers) and can confirm that #135 fixes it for me.

FYI it doesn't actually fix the bug that oscarfv reported (if the timer fires when a different buffer is current, then the timer function never reindents / removes itself). However, at least in my everyday use, the current-buffer mismatch only causes one long-lived timer (attached to the org src emacs lisp fontification buffer) which isn't that annoying. I've opened a PR for that fix.

[oscarfv]

This is what I was using since I reported the bug (still not rebased against the most recent upstream changes):

diff --git a/singles/aggressive-indent.el b/singles/aggressive-indent.el
index e4b29bb..a130c89 100644
--- a/singles/aggressive-indent.el
+++ b/singles/aggressive-indent.el
@@ -422,6 +422,16 @@ typing, try tweaking this number."
 (defvar-local aggressive-indent--idle-timer nil
   "Idle timer used for indentation")

+(defun aggressive-indent--cancel-timer ()
+  (if (timerp aggressive-indent--idle-timer)
+      (progn
+   (cancel-timer aggressive-indent--idle-timer)
+   (setq aggressive-indent--idle-timer nil))
+    ;; For an unknown reason, sometimes timers are not canceled and
+    ;; accumulate on the list of active timers, slowing down Emacs.
+    ;; This ensures that any timer we created is canceled:
+    (cancel-function-timers 'aggressive-indent--indent-if-changed)))
+
 ;; Ripped from Emacs 27.0 subr.el.
 ;; See Github Issue#111 and Emacs bug#31692.
 (defmacro aggressive-indent--while-no-input (&rest body)
@@ -462,21 +472,19 @@ If BODY finishes, `while-no-input' returns whatever value BODY produced."
 (defun aggressive-indent--indent-if-changed ()
   "Indent any region that changed in the last command loop."
   (if (not (buffer-live-p (current-buffer)))
-      (cancel-timer aggressive-indent--idle-timer)
+      (aggressive-indent--cancel-timer)
     (when (and aggressive-indent-mode aggressive-indent--changed-list)
       (save-excursion
         (save-selected-window
           (aggressive-indent--while-no-input
             (aggressive-indent--proccess-changed-list-and-indent))))
-      (when (timerp aggressive-indent--idle-timer)
-        (cancel-timer aggressive-indent--idle-timer)))))
+      (aggressive-indent--cancel-timer))))

 (defun aggressive-indent--keep-track-of-changes (l r &rest _)
   "Store the limits (L and R) of each change in the buffer."
   (when aggressive-indent-mode
     (push (list l r) aggressive-indent--changed-list)
-    (when (timerp aggressive-indent--idle-timer)
-      (cancel-timer aggressive-indent--idle-timer))
+    (aggressive-indent--cancel-timer)
     (setq aggressive-indent--idle-timer
           (run-with-idle-timer aggressive-indent-sit-for-time t #'aggressive-indent--indent-if-changed))))

@@ -511,8 +519,7 @@ If BODY finishes, `while-no-input' returns whatever value BODY produced."
         (add-hook 'after-revert-hook #'aggressive-indent--clear-change-list nil 'local)
         (add-hook 'before-save-hook #'aggressive-indent--proccess-changed-list-and-indent nil 'local))
     ;; Clean the hooks
-    (when (timerp aggressive-indent--idle-timer)
-      (cancel-timer aggressive-indent--idle-timer))
+    (aggressive-indent--cancel-timer)
     (remove-hook 'after-change-functions #'aggressive-indent--keep-track-of-changes 'local)
     (remove-hook 'after-revert-hook #'aggressive-indent--clear-change-list 'local)
     (remove-hook 'before-save-hook #'aggressive-indent--proccess-changed-list-and-indent 'local)

[Malabarba]

Sorry for the long silence. I've merged both PRs. Let's hope that fixes the problem for good. Let me know how it works for you.