Merge pull request #180 from Univers42/fix/rl-saved-line-leak

fix(editor): free the two undo lists readline 8.1 drops on the floor

Author: LESdylan

incs/history.h

@@ -54,6 +54,7 @@ void		hist_rehome(t_shell *state);
    keeps on an edited entry (history_rl.c). Never call the raw ones. */
 void		hist_rl_drop(int idx);
 void		hist_rl_clear(void);
+void		hist_rl_reclaim_saved(void);
 void		init_history(t_shell *state);
 void		free_hist(t_shell *state);
 void		parse_history_file(t_shell *state);

src/infrastructure/history_rl.c

@@ -61,6 +61,31 @@ void	hist_rl_drop(int idx)
 		undo_list_free((UNDO_LIST *)free_history_entry(e));
 }
 
+/* Before each read: free the undo list readline parked with its saved
+** copy of the typed line.
+**
+** Walking the history from a typed line (previous-history: Ctrl-P, or Up
+** with prefix search off) makes readline save that line, with its undo
+** list as the entry's data, so Down can bring it back. When the line ends
+** elsewhere -- Enter or ^C on a recalled entry -- the saved copy outlives
+** the read, and the next readline() frees it without the undo list:
+** readline 8.1 (Ubuntu 22.04's) leaked the typed text's undo records once
+** per such line, which ASan reported at exit.
+**   rl_maybe_unsave_line() is the public way to take that list back into
+** rl_undo_list, and rl_free_undo_list() frees it as readline does, also
+** dropping any history entry still pointing at it. Unsave rings the bell
+** when nothing is saved, so the save before it makes sure something is.
+** Both rewrite rl_line_buffer, which is harmless here: the read that
+** follows starts from an empty line. */
+void	hist_rl_reclaim_saved(void)
+{
+	if (!rl_line_buffer)
+		return ;
+	rl_maybe_save_line();
+	rl_maybe_unsave_line();
+	rl_free_undo_list();
+}
+
 /* clear_history(), freeing the undo lists it cannot. */
 void	hist_rl_clear(void)
 {

src/infrastructure/rl_private.h

@@ -165,6 +165,7 @@ int		rl_shell_leave(t_shell *state, t_rl_bracket *b);
 void	rl_prerow_arm(t_string *rows);
 FILE	*rl_out_open(void);
 int		rl_getc_hook(FILE *stream);
+int		rl_nsearch_reclaim(int c);
 int		rl_idle_timeout(void);
 int		rl_idle_fd(void);
 void	rl_idle_event(int r);

src/platform/posix/rl_getc.c

@@ -128,7 +128,7 @@ int	rl_getc_hook(FILE *stream)
 	{
 		rl_take_signals();
 		if (rl_should_abort())
-			return (rl_abort_value());
+			return (rl_nsearch_reclaim(rl_abort_value()));
 		rl_block(&old);
 		r = -1;
 		if (!rl_pending_signal() && !rl_should_abort())
@@ -141,5 +141,5 @@ int	rl_getc_hook(FILE *stream)
 	}
 	if (r == '\r' || r == '\n')
 		rl_suggest_unpaint();
-	return (r);
+	return (rl_nsearch_reclaim(r));
 }

src/platform/posix/rl_inproc.c

@@ -58,6 +58,7 @@ int	rl_read_inproc(t_shell *state, char *prompt)
 	char	*row;
 	char	*line;
 
+	hist_rl_reclaim_saved();
 	row = rl_editor_enter(state, prompt);
 	line = readline(row);
 	rl_editor_exit(state);

src/platform/posix/rl_nsearch.c

@@ -0,0 +1,39 @@
+/* ************************************************************************** */
+/*                                                                            */
+/*                                                        :::      ::::::::   */
+/*   rl_nsearch.c                                       :+:      :+:    :+:   */
+/*                                                    +:+ +:+         +:+     */
+/*   By: dlesieur <dlesieur@student.42.fr>          +#+  +:+       +#+        */
+/*                                                +#+#+#+#+#+   +#+           */
+/*   Created: 2026/10/18 03:40:00 by dlesieur          #+#    #+#             */
+/*   Updated: 2026/10/18 03:40:00 by dlesieur         ###   ########.fr       */
+/*                                                                            */
+/* ************************************************************************** */
+
+#include "rl_private.h"
+
+/* The byte (or EOF / READERR) the reader is about to hand readline, after
+** freeing the search string's undo list when that byte ends a
+** non-incremental search (Alt-N / Alt-P, stock) without searching.
+**
+** The search reads its string into the line buffer with an undo list of
+** its own: it saves the line, sets rl_undo_list to NULL and inserts. On
+** Enter it frees that list before searching; on abort it puts the saved
+** line back, overwriting rl_undo_list, and readline 8.1 (Ubuntu 22.04's)
+** loses the list -- a leak per aborted search, reported by ASan at exit.
+** Freeing it here, before readline sees the key, leaves nothing to lose,
+** and a readline that frees it itself finds NULL.
+**
+** Caveat: the abort keys are readline 8.1's (search.c,
+** _rl_nsearch_dispatch): ^G, ^C, Rubout or ^H on an empty string, and a
+** failed read. A key a later readline adds to that list leaks again; a
+** key it drops loses only the undo history of the search string. */
+int	rl_nsearch_reclaim(int c)
+{
+	if (!RL_ISSTATE(RL_STATE_NSEARCH))
+		return (c);
+	if (c == '\a' || c == 3 || c < 0
+		|| ((c == '\b' || c == 127) && rl_point == 0))
+		rl_free_undo_list();
+	return (c);
+}

tests/rl_undo_leak_test.py

@@ -0,0 +1,103 @@
+#!/usr/bin/env python3
+"""No undo list leaks out of readline's history walk or its aborted search.
+
+Two places where readline 8.1 (Ubuntu 22.04's) drops an undo list it owns,
+each once per line, reported by LeakSanitizer when the shell exits:
+
+  saved line  type, then walk the history with previous-history (Ctrl-P, or
+              Up with prefix search off) and end the line on the recalled
+              entry: Enter, ^C or ^G. The next read freed readline's saved
+              copy of the typed line, but not its undo list.
+              Fixed by hist_rl_reclaim_saved() (history_rl.c).
+  search      Alt-P / Alt-N (stock non-incremental search), type a string,
+              abort with ^G or Rubout on an empty string: readline restored
+              the line over the search string's undo list.
+              Fixed by rl_nsearch_reclaim() (rl_nsearch.c).
+
+Each case is a fresh shell (in-process reader: the forked one discards its
+heap with the child) run with detect_leaks=1; it must exit without a
+report. The `works/` cases check the keys still do their job. On a binary
+built without ASan nothing can be measured: SKIP, not pass.
+"""
+import os
+import sys
+import time
+
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.abspath(__file__)),
+                                "helpers"))
+from ptyexpect import Tty  # noqa: E402
+
+ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+SHELL = os.path.abspath(sys.argv[1] if len(sys.argv) > 1 else
+                        os.path.join(ROOT, "build", "bin", "hellish"))
+FAILS = []
+UP, DOWN = "\x1b[A", "\x1b[B"
+ALT_P, ALT_N = "\x1bp", "\x1bn"
+HISTORY = ["echo alpha", "echo beta"]
+
+
+def check(name, ok, detail=""):
+    print(("ok   " if ok else "FAIL ") + name)
+    if not ok:
+        FAILS.append(name)
+        print("     " + detail)
+
+
+def session(name, steps, want=None):
+    """A shell with HISTORY run, then each key group of `steps` sent and
+    given time to land, then exit. No leak report allowed; `want`, when
+    given, must be printed by the line the keys ran."""
+    t = Tty(SHELL, rc="PS1='P$ '\nHISTFILE=/dev/null\n",
+            env={"ASAN_OPTIONS": "detect_leaks=1",
+                 "HELLISH_HISTORY_PREFIX": "0"})
+    ok = t.expect(b"P$ ")
+    for c in HISTORY:
+        t.send(c + "\r")
+        ok = ok and t.expect(b"P$ ")
+    mark = len(t.out)
+    for keys in steps:
+        t.send(keys)
+        t._read(0.25)
+    if want is not None:
+        ok = ok and t.expect(b"\n" + want.encode() + b"\r\n")
+    t.send("\x03")
+    ok = ok and t.expect(b"P$ ")
+    t.send("exit\r")
+    # LeakSanitizer reports as the shell exits: read until the pty closes,
+    # so the report is in t.out before anything is decided.
+    end = time.time() + 10
+    while t._read(0.2) and time.time() < end:
+        pass
+    st = t.close()
+    leak = b"LeakSanitizer" in t.out
+    check(name, ok and not leak and os.WIFEXITED(st),
+          repr(t.since(mark)[-400:]))
+
+
+def main():
+    with open(SHELL, "rb") as f:
+        if b"AddressSanitizer" not in f.read():
+            print("SKIP %s has no ASan: leaks cannot be seen" % SHELL)
+            return 0
+    session("saved/up-then-enter", ["echo", UP, "\r"])
+    session("saved/up-then-ctrl-c", ["echo", UP, "\x03"])
+    session("saved/up-then-ctrl-g", ["ec", UP, "\x07"])
+    session("saved/ctrl-p-then-enter", ["echo", "\x10", "\r"])
+    session("saved/ctrl-p-walk-back-and-forth",
+            ["echo", "\x10", "\x10", "\x0e", "\x10", "\r"])
+    session("search/alt-p-abort-ctrl-g", [ALT_P, "ech", "\x07"])
+    session("search/alt-n-abort-ctrl-g", [ALT_N, "ech", "\x07"])
+    session("search/alt-p-rubout-past-empty", [ALT_P, "e", "\x7f", "\x7f"])
+    session("search/typed-line-then-abort", ["ls", ALT_P, "ech", "\x07"])
+    session("search/typed-then-alt-p-enter", ["ech", ALT_P, "\r"])
+    session("works/up-down-brings-the-typed-line-back",
+            ["echo typed", UP, DOWN, "\r"], want="typed")
+    session("works/alt-p-search-runs-the-match",
+            [ALT_P, "alph", "\r", "\r"], want="alpha")
+    session("works/aborted-search-restores-the-line",
+            ["echo kept", ALT_P, "x", "\x07", "\r"], want="kept")
+    print("%d failed" % len(FAILS))
+    return 1 if FAILS else 0
+
+
+sys.exit(main())