fix: memory leak & freezes on native-comp+pgtk builds

b7f84bd introduced a nasty regression that caused an infinite loop and
runaway memory usage on some pgtk+native-comp builds of Emacs when it
attempted to perform deferred native compilation of your packages. This
would make Emacs unusable and, if left alone, could even crash your
system.

The only Emacs builds I'm certain are affected are derived from
flatwhatson/emacs (like emacs-pgtk-native-comp on Guix and Arch Linux in
particular). 28.1 stable and master (on emacs-mirror/emacs@e13509468b)
are unaffected.

It appears that some, earlier pgtk builds stack idle timers differently.
I'm not entirely sure how, because it doesn't manifest in more recent
builds of Emacs, and I'm already burnt out on debugging this, but here's
how Doom encountered it:

Doom has an incremental package loader; it loads packages, piecemeal,
after Emacs has been idle for 2s, then again every 0.75s until it
finishes or the user sends input (then it waits another 2s before
starting again). However, if at any time the iloader detected that
native-compilation is in progress, it waits 2s before trying
again (repeat, until native-comp is done). But here's the catch, given
the following:

  (run-with-idle-timer
   2 nil (lambda ()
           (run-with-idle-timer 1 nil (lambda () (message "hi")))))

I had assumed "hi" would be emitted after 3 seconds (once idle), but
instead it is emitted after 2. Like this, Doom's iloader would elapse
one idle timer directly into another, ad infinitum, until Emacs was
forcibly killed.

By switching to run-at-time and employing my own rudimentary idle timer,
I avoid this issue. Also, the iloader no longer needs to be considerate
of native-comp, because the latter does its own rate-limiting controlled
by native-comp-async-jobs-number.

Amend: b7f84bdd01
This commit is contained in:
Henrik Lissner 2022-09-10 01:01:28 +02:00
parent 3fe81f4291
commit 46e23f37ba
No known key found for this signature in database
GPG key ID: B60957CA074D39A3
2 changed files with 39 additions and 42 deletions

View file

@ -84,7 +84,7 @@ If you want to disable incremental loading altogether, either remove
`doom-incremental-first-idle-timer' to nil. Incremental loading does not occur `doom-incremental-first-idle-timer' to nil. Incremental loading does not occur
in daemon sessions (they are loaded immediately at startup).") in daemon sessions (they are loaded immediately at startup).")
(defvar doom-incremental-first-idle-timer (if (featurep 'native-compile) 3.0 2.0) (defvar doom-incremental-first-idle-timer 2.0
"How long (in idle seconds) until incremental loading starts. "How long (in idle seconds) until incremental loading starts.
Set this to nil to disable incremental loading.") Set this to nil to disable incremental loading.")
@ -100,36 +100,40 @@ Set this to nil to disable incremental loading.")
If NOW is non-nil, load PACKAGES incrementally, in `doom-incremental-idle-timer' If NOW is non-nil, load PACKAGES incrementally, in `doom-incremental-idle-timer'
intervals." intervals."
(if (not now) (let ((gc-cons-threshold most-positive-fixnum))
(appendq! doom-incremental-packages packages) (if (not now)
(if (and packages (bound-and-true-p comp-files-queue)) (cl-callf append doom-incremental-packages packages)
(run-with-idle-timer doom-incremental-idle-timer
nil #'doom-load-packages-incrementally
packages t)
(while packages (while packages
(let* ((gc-cons-threshold most-positive-fixnum) (let ((req (pop packages))
(req (pop packages))) idle-time)
(unless (featurep req) (if (featurep req)
(doom-log "Incrementally loading %s" req) (doom-log "[ILoader] Already loaded: %s (%d left)" req (length packages))
(condition-case-unless-debug e (condition-case-unless-debug e
(or (while-no-input (and
;; If `default-directory' is a directory that doesn't exist (or (null (setq idle-time (current-idle-time)))
;; or is unreadable, Emacs throws up file-missing errors, so (< (float-time idle-time) doom-incremental-first-idle-timer)
;; we set it to a directory we know exists and is readable. (not
(let ((default-directory doom-emacs-dir) (while-no-input
(inhibit-message t) (doom-log "[ILoader] Loading: %s (%d left)" req (length packages))
file-name-handler-alist) ;; If `default-directory' doesn't exist or is
(require req nil t)) ;; unreadable, Emacs throws file errors.
t) (let ((default-directory doom-emacs-dir)
(push req packages)) (inhibit-message t)
(file-name-handler-alist
(list (rassq 'jka-compr-handler file-name-handler-alist))))
(require req nil t)
t))))
(push req packages))
(error (error
(message "Failed to load %S package incrementally, because: %s" (message "Error: failed to incrementally load %S because: %s" req e)
req e))) (setq packages nil)))
(if (not packages) (if (null packages)
(doom-log "Finished incremental loading") (doom-log "[ILoader] Finished!")
(run-with-idle-timer doom-incremental-idle-timer (run-at-time (if idle-time
nil #'doom-load-packages-incrementally doom-incremental-idle-timer
packages t) doom-incremental-first-idle-timer)
nil #'doom-load-packages-incrementally
packages t)
(setq packages nil)))))))) (setq packages nil))))))))
(defun doom-load-packages-incrementally-h () (defun doom-load-packages-incrementally-h ()

View file

@ -321,23 +321,16 @@ users).")
;; ;;
;;; Native Compilation support (http://akrl.sdf.org/gccemacs.html) ;;; Native Compilation support (http://akrl.sdf.org/gccemacs.html)
(when (featurep 'native-compile) (when (boundp 'native-comp-eln-load-path)
;; Enable deferred compilation and disable ahead-of-time compilation, so we
;; don't bog down the install process with an excruciatingly long compile
;; times. It will mean more CPU time at runtime, but given its asynchronosity,
;; this is acceptable.
(setq native-comp-deferred-compilation t
straight-disable-native-compile t)
;; Suppress compiler warnings, to avoid inundating users will popups. They
;; don't cause breakage, so it's not worth dedicating screen estate to them.
(setq native-comp-async-report-warnings-errors init-file-debug
native-comp-warning-on-missing-source init-file-debug)
;; Don't store eln files in ~/.emacs.d/eln-cache (where they can easily be ;; Don't store eln files in ~/.emacs.d/eln-cache (where they can easily be
;; deleted by 'doom upgrade'). ;; deleted by 'doom upgrade').
;; REVIEW Use `startup-redirect-eln-cache' when 28 support is dropped ;; REVIEW Use `startup-redirect-eln-cache' when 28 support is dropped
(add-to-list 'native-comp-eln-load-path (expand-file-name "eln/" doom-cache-dir))) (add-to-list 'native-comp-eln-load-path (expand-file-name "eln/" doom-cache-dir))
;; UX: Suppress compiler warnings and don't inundate users with their popups.
;; They are rarely more than warnings, so are safe to ignore.
(setq native-comp-async-report-warnings-errors init-file-debug
native-comp-warning-on-missing-source init-file-debug))
;; ;;