persp-current-buffers*: read frame-global buffers without switching perspectives - #233
Merged
Merged
Conversation
with-perspective runs persp-switch (twice — there and back), and each switch restores a window configuration. Callers of persp-current-buffers* / persp-is-current-buffer / persp-buffer-filter treat them as pure predicates, so this side effect is surprising — and fatal when a predicate runs inside redisplay: a menu-bar item's :enable form that consults a buffer predicate (e.g. CIDER's menu via a sesman-to-perspective filter) triggers delete-other-windows from within update_menu_bar, reallocating the window's glyph matrices while display_line iterates over them. Emacs 31.0.90 segfaults in set_cursor_from_row / display_line (report to bug-gnu-emacs in flight). Read the frame global perspective's buffer list directly from perspectives-hash instead. persp-get-scratch-buffer already accepts the perspective name, so behavior is unchanged. copy-sequence keeps the destructive delete-dups off the global perspective's live buffer list, which the previous code exposed as the shared tail of append. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01F3SLHGSnQuGu2QukBqs1wd
Collaborator
|
Looks solid. That’s some impressive lldb work by Claude. Merged. |
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
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.
Problem
persp-current-buffers*withINCLUDE-GLOBALreads the frame global perspective's buffer list viawith-perspective, which runspersp-switchtwice (there and back). Each switch restores a window configuration. Callers ofpersp-current-buffers*/persp-is-current-buffer/persp-buffer-filterreasonably treat them as pure predicates, so this side effect is surprising — and it turns out to be fatal in one realistic scenario:Emacs evaluates menu-bar
:enableforms during redisplay (update_menu_bar → menu_item_eval_property). If such a form reaches a perspective buffer predicate (in my case: CIDER's menu:enable (cider-connected-p)→ sesman session lookup → a sesman-to-perspective filter →persp-is-current-buffer buf t), thewith-perspectiveround-trip executesdelete-other-windowsmid-redisplay, which callsadjust_frame_glyphsand reallocates the window's glyph matrices whiledisplay_lineholds pointers into them. Emacs 31.0.90 segfaults 100% reproducibly (set_cursor_from_rowcomputes a garbagew->cursor.vposfrom a stale row pointer; the nextdisplay_lineindexes ~340 GB out of bounds). I debugged this under lldb down to the exact instructions, and caught thepersp-switch → delete-other-windows → adjust_glyph_matrixchain live with a breakpoint whileredisplay_internalwas on the stack. A hardening report to bug-gnu-emacs is in flight, but the crash is trivially avoided by not switching perspectives here at all.Fix
Read the frame global perspective's buffer list directly from
perspectives-hash.persp-get-scratch-bufferalready accepts the perspective name, so the scratch-buffer exclusion behaves identically. This follows the same approach as #226 (persp-maybe-kill-buffer: avoid switching perspectives).Bonus fix: the previous code returned the global perspective's live buffer list as the shared tail of
append, exposing it to the destructivedelete-dups;copy-sequencenow protects it.Testing
make test: 32/33 pass, matching pristine HEAD on the same Emacs — the one failure (basic-persp-window-prev-buffers) pre-exists on master under Emacs 31.0.90 batch mode and is unrelated (verified by running the suite on an unpatched checkout). Byte-compiles with no new warnings. The equivalent change applied at the configuration level (checkingpersp-buffersmembership directly, bypassing thewith-perspectiveround-trip) eliminates the segfault in the originally crashing CIDER + perspective session.🤖 Generated with Claude Code
https://claude.ai/code/session_01F3SLHGSnQuGu2QukBqs1wd