Subject: bug#76538: 31.0.50; 31.0.50; 31.0.50; feature/igc: using magit-section-cycle-global (S-TAB) and magit-section-toggle (TAB) in some random ways blocks GNU Emacs.

From: Pip Cet <[email-masked]>
Date: Mon, 03 Mar 2025 14:01:06 +0000

Gerd Möllmann <[email-masked]> writes:

> Pip Cet <[email-masked]> writes:
>
>> Here's my current patch series for finalizing markers.  Still needs
>> testing to see whether it actually avoids the problem (or will once we
>> stop the world for finalizers, as we should), but it doesn't crash
>> immediately.
>>
>> BTW, I'm currently running with a background thread eagerly triggering
>> GC, which allowed me to find the missing bt.function trace, but hasn't
>> uncovered other bugs so far.  So that is concurrent GC, even if it's
>> unlikely to be a performance win right now :-)
>
> :-).
>
>> I have no idea why I hit the assert which required the second patch. It
>> seems to me like a bug in the xdisp.c code, but it's xdisp.c, so who
>> knows?
>
> Yeah, that doesn't look right. Normally, string_char_and_length should
> have been used to get characters from a C string.
>
>> Even though we now emulate the alloc.c code quite precisely, I think
>> feature/igc will accumulate many more markers than alloc.c, since weak
>> objects are collected quite rarely by MPS, IME.  Maybe we need to mark
>> "automatic" markers which are never exposed to Lisp and splat them in
>> DO_MARKERS if there are too many of them?
>>
>> A more convoluted approach would be to alternate between considering
>> markers and calculating the charpos for the "best" known marker: do one
>> marker, then one character, repeat.  That sort of thing is good for
>> theoretical complexity but rarely useful in practice...
>
> I'd like to see something not using markers at all, TBH.
>
> Otherwise, LGTM. Only thing I saw is this:
>
>> +static void
>> +finalize_marker (struct Lisp_Marker *m)
>> +{
>> +  if (m->buffer && BUF_MARKERS (m->buffer))
>                       ^^^^^^^^^^^
>                       NILP

Thanks!  The fprintf should also go, but testing revealed a somewhat
more difficult problem: when garbage_collection_messages is true, the
maybe_process_messages call in maybe_finalize can try to print to a
buffer, which is bad because we're in the middle of tearing down one.  I
think we should remove that call, to be honest, and put it in maybe_quit
instead, since some messages may cause us to print messages or run Lisp.

Alternatively, we could use pending_funcalls or a similar mechanism.

WDYT?

Pip