Subject: Re: bug#79200: 31.0.50; Duplicated elements for '#<marker at' in buffer-undo-list

From: Pip Cet <[email-masked]>
Date: Sun, 10 Aug 2025 14:56:56 +0000

"Alan Mackenzie" <[email-masked]> writes:

> Hello, Gerd.
>
> On Sun, Aug 10, 2025 at 06:55:45 +0200, Gerd Möllmann wrote:
>> Pip Cet <[email-masked]> writes:
>
>> >> - record_marker_adjustments undo.c:127
>> >>   - record_delete record_delete:187
>> >>     + casify_region casify_region:555
>> >>     + adjust_after_replace adjust_after_replace:1408
>> >>     + replace_range replace_range:1639
>> >>     + del_range_2 del_range_2:2030
>> >>     + record_change record_change:201
>
>> >> where record_marker_adjustments puts basically all present markers in a
>> >> buffer's undo list. del_range_2 I would interpret that any text deletion
>> >> might do that.
>
>> > I think it may make more sense to find out who goes around creating
>> > markers and not cleaning up after them. But I suspect many places cannot
>> > easily be fixed because the markers might be exposed to Lisp.
>
>> Probably. Good question is how to find these places, especially when
>> they are created in Lisp :-/.

I think the Lisp part is easy: we can simply capture a backtrace and
attach it when creating the marker.

>From f7a23531886fe458346f13ee08369d153b2de897 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Sun, 10 Aug 2025 14:53:49 +0000
Subject: [PATCH] store Lisp backtraces for markers

---
 src/alloc.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/src/alloc.c b/src/alloc.c
index 07ca8474bf3..c8426e3a4dc 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -3812,6 +3812,12 @@ DEFUN ("make-marker", Fmake_marker, Smake_marker, 0, 0, 0,
   p->next = NULL;
   p->insertion_type = 0;
   p->need_adjustment = 0;
+  if (!NILP (Vmarker_backtrace))
+    {
+      Lisp_Object backtrace = make_nil_vector (32);
+      get_backtrace (XVECTOR (backtrace)->contents, 32);
+      Fputhash (make_lisp_ptr (p, Lisp_Vectorlike), backtrace, Vmarker_backtrace);
+    }
   return make_lisp_ptr (p, Lisp_Vectorlike);
 }
 
@@ -3836,6 +3842,12 @@ build_marker (struct buffer *buf, ptrdiff_t charpos, ptrdiff_t bytepos)
   m->need_adjustment = 0;
   m->next = BUF_MARKERS (buf);
   BUF_MARKERS (buf) = m;
+  if (!NILP (Vmarker_backtrace))
+    {
+      Lisp_Object backtrace = make_nil_vector (32);
+      get_backtrace (XVECTOR (backtrace)->contents, 32);
+      Fputhash (make_lisp_ptr (m, Lisp_Vectorlike), backtrace, Vmarker_backtrace);
+    }
   return make_lisp_ptr (m, Lisp_Vectorlike);
 }
 
@@ -7461,6 +7473,10 @@ syms_of_alloc (void)
 If this portion is smaller than `gc-cons-threshold', this is ignored.  */);
   Vgc_cons_percentage = make_float (0.1);
 
+  DEFVAR_LISP ("marker-backtrace", Vmarker_backtrace,
+	       doc: /* */);
+  Vmarker_backtrace = CALLN (Fmake_hash_table, QCtest, Qeq, QCweakness, Qkey);
+
   DEFVAR_INT ("pure-bytes-used", pure_bytes_used,
 	      doc: /* No longer used.  */);
 
-- 
2.50.0

Use like this:

(dolist (el buffer-undo-list)
  (and (consp el)
       (markerp (car el))
       (message "%S %S" (car el) (gethash (car el) marker-backtrace))))

> I've made a start on this (see patch below).  It's mainly in the C files,
> where I grepped for Fpoint_marker and Fcopy_marker, and tidied up the
> uses which didn't tidy up their uses of markers.  But also in some Lisp
> files, which are directly part of the command loop (simple.el), or called
> from it in hooks (jit.el, electric.el).
>
> Unfortunately, this hasn't helped much.  When I type "asdfasdf" into
> *scratch* on emacs -Q, then <BACKSPACE>, I still see five point markers
> in buffer-undo-list.  If I <BACKSPACE> a second time, this adds on a
> further ten point markers to b-u-list.
>
> It's worth mentioning that these markers aren't eq to eachother.  At
> least, not all of them.  I have a strong suspicion that there are
> duplicates in the 15 markers in buffer-undo-list after my two deletions.
>
> It is unclear whether the <BACKSPACE> creates these markers, or just puts
> existing markers into buffer-undo-list.  Maybe we could do with a
> primitive in marker.c which would print out the current buffer's list of
> markers.

That also sounds like a useful debugging aid!

>
>> >> I think a garbage-collect with the old GC will clean up the undo list
>> >> eventually by calling compact_undo_list. With MPS, that doesn't work.
>> >> The cleanup in compact_undo_list cannot be used because MPS's weak
>> >> objects and GC work completely differently.
>
>> > That's my understanding, too. And it means that this probably isn't
>> > horribly important to fix on the master branch. On feature/igc, it looks
>> > like something has to be done.
>
>> >> At the moment I'm a bit out of ideas. Waiting for a divine inspiration,
>> >> so to speak.
>
>> > Maybe some markers could be marked at creation time as not being
>> > important enough to be recorded in the undo list? I'm not really sure
>> > which markers are being recorded and which need to be for undo to work.
>
> This might be a silly question, but why do we have to put markers into
> buffer-undo-list at all?  Surely they should "fix themselves" as other
> changes in the buffer get undone?

I don't know the answer. I assume it's necessary for text removals which
result in two markers sharing a position when they didn't previously, so
we can undo those properly?

>> Might be possible but also requires to find where the markers are
>> created, find out if they are nver required for undo, and so on.
>
> Here's a patch which tidies up several markers.  Unfortunately it doesn't
> seem to make a massive difference to the problem.

Have you tried applying the patch I posted earlier, too? It doesn't
appear to be included in your changes, but it made a difference here.

This is against feature/igc, but the code looks unchanged from master.

>From d5cf17f88fbba4cdfc61ebea725e12a2c8b9aa42 Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Sat, 9 Aug 2025 10:32:19 +0000
Subject: [PATCH] Detach markers explicitly so they don't wind up on the undo
 list

* src/xdisp.c (unwind_format_mode_line): Detach markers when we're
done with them.
---
 src/xdisp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/xdisp.c b/src/xdisp.c
index 432dd5dceca..2406bb6f3d9 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13828,6 +13828,7 @@ unwind_format_mode_line (Lisp_Object vector)
 
 	      current_buffer = XBUFFER (buffer);
 	      set_point_from_marker (AREF (vector, 11));
+	      detach_marker (AREF (vector, 11));
 	      ASET (vector, 11, Qnil);
 	      current_buffer = cb;
 	    }
-- 
2.50.0

Pip