Subject: Re: bug#74547: 31.0.50; igc: assertion failed in buffer.c

From: Gerd Möllmann <[email-masked]>
Date: Sun, 01 Dec 2024 14:30:37 +0100

Pip Cet <[email-masked]> writes:

> Gerd Möllmann <[email-masked]> writes:
>> Pip Cet <[email-masked]> writes:
>> Yes, exactly, json.c. First thing I saw when searching for xfree
>>
>> static void
>> json_parser_done (void *parser)
>> {
>>   struct json_parser *p = (struct json_parser *) parser;
>>   if (p->object_workspace != p->internal_object_workspace)
>>     xfree (p->object_workspace);
>>
>> That at least needs an explanation. I would have expected it to be
>> allocated as root.
>
> Well, the explanation is this comment:
>
>   /* Lisp_Objects are collected in this area during object/array
>      parsing.  To avoid allocations, initially
>      internal_object_workspace is used.  If it runs out of space then
>      we switch to allocated space.  Important note: with this design,
>      GC must not run during JSON parsing, otherwise Lisp_Objects in
>      the workspace may get incorrectly collected. */

That explains it, indeed :-(.

>
> Obviously, we cannot make any such guarantees when MPS is in use. (I
> don't think we can make the guarantee when MPS is not in use, but I'm
> not totally certain; we certainly allocate strings while parsing JSON,
> which is sufficient to trigger GC in the MPS case).

If json.c calls something like maybe_quit, which I's expect it must,
then GC can indeed happen. See bug#56108 for an example in the regexp
code found with ASAN. It's not as risky in the old code as with
concurrent GC, but anyway.

>
> Note that the json_parser object itself is fine (it's allocated on the
> stack, thus marked ambiguously), it's only in the case that we create
> more than 64 Lisp_Object values when parsing a single JSON document that
> we end up with untraced references on the heap.
>
> I don't know whether it's likely that that was what happened to Oscar.
> My gut feeling is 64 objects would be easily reached by LSP messages,
> but I'd need more time to test.
>
> Anyway, here's a patch which might help:
>
> commit c175744f2172ba3405ae98eb3575b2bf4adadfa4
> Author: Pip Cet <[email-masked]>
> Date:   Sun Dec 1 12:46:08 2024 +0000

Very nide, thank you!