Subject: Re: bug#79014: 31.0.50; igc: infinite loop

From: Pip Cet <[email-masked]>
Date: Mon, 14 Jul 2025 15:08:23 +0000

Óscar Fuentes <[email-masked]> writes:

> Eli Zaretskii <[email-masked]> writes:
>
>>> > Any idea what it was trying to warn about in display-warning frames?
>>>
>>> No, sorry.
>>>
>>> > I suspect it's some kind of memory-full situation.
>>>
>>> I'll mention this in case it is relevant: Emacs was using about 2.8 GB
>>> of RSS and that machine has 64 GB of RAM, almost all free.
>>>
>>> > Do you still have that session in GDB?
>>>
>>> No :-(
>>
>> Then I don't see how we could do anything with this report.  Too bad.
>> Please in the future try to leave the crashed session inside GDB at
>> least for a while.
>
> That's a remote machine that runs Emacs as a daemon. Probably I could
> figure out how to run another remote instance without killing the
> previous one, but I had urgent work to do.

>> Unless someone has ideas how to make some progress with this bug, I
>> will soon close it as unreproducible.
>
> Let's see what Pip has to say.

I'll have to look up the conversation, but IIRC, this is a clear bug:
igc_on_idle shouldn't always start on the same buffer, and it should not
call itself recursively; compact_buffer should not attempt to truncate
an undo list in a recursive call, either.  I'm pretty sure I suggested
moving this line

      BUF_COMPACT (buffer) = BUF_MODIFF (buffer);

in compact_buffer to happen before we call truncate_undo_list, which
would prevent this infloop, but there's more than one thing to be fixed
here.

> For starters, does it make sense for igc_on_idle to be recursive?

No, I don't think it does. If I'm reading the code correctly,
display-warning calls (sit-for 0), and that probably shouldn't cause us
to become idle at all.

> Can't that function detect that is being called recursively and just
> return?

I'm not sure I have a good idea of how to fix this yet. igc_on_idle
appears to be called in the wrong place if there's a possibility it
calls out to Lisp, too.

> Another thought: does the time consumed by igc_on_idle count as idle
> time for idle timers?

I'd like to rephrase that a little: why isn't igc_on_idle an idle timer?
There's already some logic in keyboard.c to run idle timers at the
correct time, when actually idle, so we could reuse that for the idle
work that igc.c needs to do.

For now, though, let's do this (on feature/igc):

>From 41d3fdf4c394c14e3beed1eda3caec6b8e57cb2f Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Date: Mon, 14 Jul 2025 15:05:05 +0000
Subject: [PATCH] Avoid infloops during buffer compaction (bug#79014)

* src/buffer.c (compact_buffer): Set the 'BUF_COMPACT' timestamp
before entering truncate_undo_list; that way, if truncate_undo_list
calls this function recursively, the same buffer will not be
considered for compaction twice.
---
 src/buffer.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/buffer.c b/src/buffer.c
index b85394cff3b..e474b8f9dfe 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -1866,6 +1866,10 @@ compact_buffer (struct buffer *buffer)
       && (buffer->base_buffer == NULL)
       && (BUF_COMPACT (buffer) != BUF_MODIFF (buffer)))
     {
+      /* This needs to be first: truncate_undo_list can call out to
+	 Lisp, which may result in this function being reentered.  Make
+	 sure we don't infloop in this case.  */
+      BUF_COMPACT (buffer) = BUF_MODIFF (buffer);
       /* If a buffer's undo list is Qt, that means that undo is
 	 turned off in that buffer.  Calling truncate_undo_list on
 	 Qt tends to return NULL, which effectively turns undo back on.
@@ -1885,7 +1889,6 @@ compact_buffer (struct buffer *buffer)
 	  if (BUF_GAP_SIZE (buffer) > size)
 	    make_gap_1 (buffer, -(BUF_GAP_SIZE (buffer) - size));
 	}
-      BUF_COMPACT (buffer) = BUF_MODIFF (buffer);
     }
 }
 
-- 
2.50.0