Fix async diff buffer race causing wrong/missing highlights and errors - #281
Closed
LemonBreezes wants to merge 1 commit into
Closed
Fix async diff buffer race causing wrong/missing highlights and errors#281LemonBreezes wants to merge 1 commit into
LemonBreezes wants to merge 1 commit into
Conversation
When `diff-hl-update-async' is enabled, diffs run as background processes
and finish in a sentinel. All updates shared the global buffer names
" *diff-hl* " and " *diff-hl-reference* ", so several updates in flight at
once (e.g. a project/workspace switch refreshing many buffers, or the
flydiff idle timer) would clobber or empty each other's diff output.
Worse, if such a shared buffer was killed before its continuation ran,
`diff-hl--resolve' dereferenced it by name unguarded on Emacs 31's
`vc-run-delayed-success' path, signaling
error "No buffer named *diff-hl-reference* "
Give every update its own freshly generated diff buffers, and have the
resolvers kill them once parsed (including on the process-killed path, so
a sibling buffer can't leak). Also guard the buffer access in both
`diff-hl--resolve' branches so a buffer killed out from under us degrades
to "no highlights this cycle" instead of erroring.
The async test polled the old fixed buffer names to wait for completion;
update it to wait on any live diff-hl process.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
|
Hi! If you don't have a specific reproduction scenario, you might want to ask your coding agent to generate one just based on the brief description and the error text from the messages buffer - they can be handy at that. |
LemonBreezes
marked this pull request as ready for review
May 31, 2026 22:56
Author
;; Self-contained reproduction for the bug
;;
;; error "No buffer named *diff-hl-reference* "
;;
;; signalled from a diff-hl async update finishing in a process sentinel.
;;
;; It bootstraps straight.el into a throwaway tree, installs diff-hl from a
;; chosen repo/branch, opens a Git-tracked file with both staged and unstaged
;; changes, kicks off an *asynchronous* diff-hl update, and lets a concurrent
;; buffer kill (what a project/workspace switch does to the shared, fixed-name
;; diff buffers) race with the update's still-pending continuation.
;;
;; HOW TO RUN
;;
;; Upstream master (reproduces the bug):
;; emacs -Q --batch -l repro-async-race.el
;;
;; The proposed fix (no error):
;; DIFFHL_REPO=LemonBreezes/diff-hl \
;; DIFFHL_BRANCH=fix-async-diff-buffer-race \
;; emacs -Q --batch -l repro-async-race.el
;;
;; Each (repo,branch) gets its own isolated straight tree under
;; /tmp/diff-hl-repro/, so this never touches your real Emacs config. Exit
;; code is 1 when the bug reproduces, 0 when it does not.
;;; --- Pick which diff-hl to test -------------------------------------------
(defvar repro-repo (or (getenv "DIFFHL_REPO") "dgutov/diff-hl"))
(defvar repro-branch (or (getenv "DIFFHL_BRANCH") "master"))
;; Isolate everything (straight tree, repos, build) per variant.
(setq user-emacs-directory
(file-name-as-directory
(expand-file-name
(format "%s--%s"
(replace-regexp-in-string "/" "_" repro-repo)
repro-branch)
"/tmp/diff-hl-repro/")))
(make-directory user-emacs-directory t)
(setq straight-base-dir user-emacs-directory)
(message "\n==== diff-hl async race repro ====")
(message "Emacs : %s" emacs-version)
(message "diff-hl : %s @ %s" repro-repo repro-branch)
(message "tree : %s\n" user-emacs-directory)
;;; --- Bootstrap straight (verbatim from the report) ------------------------
(defvar bootstrap-version)
(let ((bootstrap-file
(expand-file-name "straight/repos/straight.el/bootstrap.el" user-emacs-directory))
(bootstrap-version 5))
(unless (file-exists-p bootstrap-file)
(with-current-buffer
(url-retrieve-synchronously "https://raw.githubusercontent.com/raxod502/straight.el/develop/install.el"
'silent 'inhibit-cookies)
(goto-char (point-max))
(eval-print-last-sexp)))
(load bootstrap-file nil 'nomessage))
;; Install diff-hl from the chosen repo/branch instead of the default recipe.
(straight-use-package
`(diff-hl :type git :host github :repo ,repro-repo :branch ,repro-branch))
(require 'diff-hl)
;; Report exactly which commit got built, so the comparison is unambiguous.
(let ((default-directory
(expand-file-name "straight/repos/diff-hl/" user-emacs-directory)))
(message "diff-hl commit: %s"
(string-trim
(with-output-to-string
(with-current-buffer standard-output
(call-process "git" nil t nil "rev-parse" "--short" "HEAD"))))))
;;; --- Record any error that escapes the async update ----------------------
(defvar repro-errors nil)
;; diff-hl--resolve is where the doomed `(with-current-buffer
;; " *diff-hl-reference* " ...)` lives on the Emacs 31+ `vc-run-delayed-success'
;; path. It runs inside a process sentinel. In a live session the error
;; escapes as a top-level "error in process sentinel: No buffer named
;; " *diff-hl-reference* "" (exactly the reported backtrace); here we catch it
;; so the script can print a clean verdict and exit code instead of aborting.
(advice-add 'diff-hl--resolve :around
(lambda (orig &rest args)
(condition-case err
(apply orig args)
(error
(push err repro-errors)
(message "error in process sentinel: %s" (error-message-string err))
nil))))
;;; --- Build a tiny Git repo with staged + unstaged changes ----------------
(defun repro-git (dir &rest args)
(let ((default-directory dir))
(apply #'process-file "git" nil nil nil args)))
(let* ((repo (file-name-as-directory (make-temp-file "diff-hl-repro-repo" t)))
(file (expand-file-name "foo.txt" repo)))
(repro-git repo "init" "-q")
(repro-git repo "config" "user.email" "repro@example.com")
(repro-git repo "config" "user.name" "Repro")
;; Committed baseline.
(with-temp-file file
(dotimes (i 60) (insert (format "line %d\n" i))))
(repro-git repo "add" "foo.txt")
(repro-git repo "commit" "-qm" "base")
;; A staged edit ...
(with-temp-file file
(dotimes (i 60) (insert (format "line %d\n" i)))
(goto-char (point-min)) (forward-line 5)
(insert "STAGED edit\n"))
(repro-git repo "add" "foo.txt")
;; ... and an unstaged edit on top. With `diff-hl-show-staged-changes' nil
;; this makes diff-hl compute BOTH a working diff (" *diff-hl* ") and a
;; reference diff (" *diff-hl-reference* ").
(with-temp-file file
(insert (with-temp-buffer (insert-file-contents file) (buffer-string)))
(goto-char (point-min)) (forward-line 30)
(insert "UNSTAGED edit\n"))
(find-file file)
(setq-local diff-hl-update-async t) ; the async path
(setq-local diff-hl-show-staged-changes nil) ; force the reference diff
(diff-hl-mode 1)
;; Kick off an async update: this spawns the background "diff-files" and
;; "diff-index" git processes that write into the shared, fixed-name buffers.
(diff-hl-update)
;; Now race a concurrent buffer kill against the still-pending update -- this
;; is what a project/workspace switch (or any churn that refreshes many
;; buffers at once) does to the *globally shared* " *diff-hl-reference* "
;; buffer while another buffer's update is mid-flight.
(when-let* ((rb (get-buffer " *diff-hl-reference* ")))
(kill-buffer rb))
;; Drain the background processes so the working diff's continuation runs.
;; On upstream it then does `(with-current-buffer " *diff-hl-reference* " ...)'
;; on the now-dead buffer and signals the reported error.
(dotimes (_ 250)
(accept-process-output nil 0.02)))
;;; --- Verdict --------------------------------------------------------------
(message "\n==== RESULT ====")
(if repro-errors
(progn
(message "BUG REPRODUCED: %S" (car (last repro-errors)))
(message "(%d error(s) escaped the async diff-hl update)" (length repro-errors))
(kill-emacs 1))
(message "No error escaped the async update -- this build is NOT affected.")
(kill-emacs 0))
;;; repro-async-race.el ends here |
Owner
|
Thanks, this should resolve it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've been getting recurring errors with
diff-hlfor years and I'm hoping this fixes it: