Repository navigation
fix(editor): free the two undo lists readline 8.1 drops on the floor - #180
Merged
Merged
Conversation
ASan reported a leak at exit after an ordinary editing session: 32 bytes plus the typed text, allocated by readline's rl_add_undo. Two paths in readline 8.1 (Ubuntu 22.04's, what the 42 machines link) lose an undo list once per line, and the in-process reader made them the shell's: - Saved line. Walking the history from a typed line with previous-history (Ctrl-P, or Up when prefix search is off) saves that line, with its undo list as data, so Down can bring it back. Ended on the recalled entry (Enter, ^C, ^G), the saved copy outlives the read and the next readline() frees it without its data. hist_rl_reclaim_saved() now runs before each read and takes the list back with public calls only: rl_maybe_save_line() so there is a saved line (unsave rings the bell otherwise), rl_maybe_unsave_line() to move its data into rl_undo_list, rl_free_undo_list() to free it, which also clears any history entry still pointing at it. - Aborted search. Alt-N / Alt-P (stock non-incremental search) reads its string with an undo list of its own; aborting restores the saved line over rl_undo_list. rl_nsearch_reclaim() frees that list in the reader when the byte readline is about to get ends the search: ^G, ^C, Rubout or ^H on an empty string, a failed read. A readline that frees it itself finds NULL. Marked Caveat: the abort keys mirror 8.1's search.c. Both predate #136/#140 (same report on 134ac5a); prefix search made the first one the everyday path on a non-PIE build, where Up was plain previous-history. tests/rl_undo_leak_test.py runs 10 leaking key sequences, each in a fresh shell with detect_leaks=1, plus 3 checks that the keys still work (Up then Down restores the typed line, Alt-P finds, ^G restores). Before: 11 failed; after: 0. SKIP on a binary built without ASan.
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
1 participant
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.
ASan reported a leak at exit after normal editing (32 bytes + typed text, from
rl_add_undo). Two readline 8.1 paths lose an undo list once per line; the in-process reader made them ours. Both predate #136/#140.readline()freed the saved copy but not its undo list.hist_rl_reclaim_saved()takes it back before each read with public calls only (rl_maybe_save_line/rl_maybe_unsave_line/rl_free_undo_list).rl_nsearch_reclaim()frees it in the reader on the aborting byte (Caveat-marked: the abort keys mirror 8.1'ssearch.c).tests/rl_undo_leak_test.py: 10 leaking sequences withdetect_leaks=1+ 3 "keys still work" checks. Before 11 failed, after 0. SKIP without ASan.🤖 Generated with Claude Code