Skip to content

Commit 46e23f3

Browse files
committed
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@e13509468b7c) 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: b7f84bd
1 parent 3fe81f4 commit 46e23f3

File tree

2 files changed

+39
-42
lines changed

2 files changed

+39
-42
lines changed

lisp/doom-start.el

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ If you want to disable incremental loading altogether, either remove
8484
`doom-incremental-first-idle-timer' to nil. Incremental loading does not occur
8585
in daemon sessions (they are loaded immediately at startup).")
8686

87-
(defvar doom-incremental-first-idle-timer (if (featurep 'native-compile) 3.0 2.0)
87+
(defvar doom-incremental-first-idle-timer 2.0
8888
"How long (in idle seconds) until incremental loading starts.
8989
9090
Set this to nil to disable incremental loading.")
@@ -100,36 +100,40 @@ Set this to nil to disable incremental loading.")
100100
101101
If NOW is non-nil, load PACKAGES incrementally, in `doom-incremental-idle-timer'
102102
intervals."
103-
(if (not now)
104-
(appendq! doom-incremental-packages packages)
105-
(if (and packages (bound-and-true-p comp-files-queue))
106-
(run-with-idle-timer doom-incremental-idle-timer
107-
nil #'doom-load-packages-incrementally
108-
packages t)
103+
(let ((gc-cons-threshold most-positive-fixnum))
104+
(if (not now)
105+
(cl-callf append doom-incremental-packages packages)
109106
(while packages
110-
(let* ((gc-cons-threshold most-positive-fixnum)
111-
(req (pop packages)))
112-
(unless (featurep req)
113-
(doom-log "Incrementally loading %s" req)
107+
(let ((req (pop packages))
108+
idle-time)
109+
(if (featurep req)
110+
(doom-log "[ILoader] Already loaded: %s (%d left)" req (length packages))
114111
(condition-case-unless-debug e
115-
(or (while-no-input
116-
;; If `default-directory' is a directory that doesn't exist
117-
;; or is unreadable, Emacs throws up file-missing errors, so
118-
;; we set it to a directory we know exists and is readable.
119-
(let ((default-directory doom-emacs-dir)
120-
(inhibit-message t)
121-
file-name-handler-alist)
122-
(require req nil t))
123-
t)
124-
(push req packages))
112+
(and
113+
(or (null (setq idle-time (current-idle-time)))
114+
(< (float-time idle-time) doom-incremental-first-idle-timer)
115+
(not
116+
(while-no-input
117+
(doom-log "[ILoader] Loading: %s (%d left)" req (length packages))
118+
;; If `default-directory' doesn't exist or is
119+
;; unreadable, Emacs throws file errors.
120+
(let ((default-directory doom-emacs-dir)
121+
(inhibit-message t)
122+
(file-name-handler-alist
123+
(list (rassq 'jka-compr-handler file-name-handler-alist))))
124+
(require req nil t)
125+
t))))
126+
(push req packages))
125127
(error
126-
(message "Failed to load %S package incrementally, because: %s"
127-
req e)))
128-
(if (not packages)
129-
(doom-log "Finished incremental loading")
130-
(run-with-idle-timer doom-incremental-idle-timer
131-
nil #'doom-load-packages-incrementally
132-
packages t)
128+
(message "Error: failed to incrementally load %S because: %s" req e)
129+
(setq packages nil)))
130+
(if (null packages)
131+
(doom-log "[ILoader] Finished!")
132+
(run-at-time (if idle-time
133+
doom-incremental-idle-timer
134+
doom-incremental-first-idle-timer)
135+
nil #'doom-load-packages-incrementally
136+
packages t)
133137
(setq packages nil))))))))
134138

135139
(defun doom-load-packages-incrementally-h ()

lisp/doom.el

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -321,23 +321,16 @@ users).")
321321
;;
322322
;;; Native Compilation support (http://akrl.sdf.org/gccemacs.html)
323323

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

342335

343336
;;

0 commit comments

Comments
 (0)