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

[Pip Cet]

Gerd Möllmann <gerd.moellmann@gmail.com> writes:
> Pip Cet <pipcet@protonmail.com> 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. */

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).

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 <pipcet@protonmail.com>
Date:   Sun Dec 1 12:46:08 2024 +0000

    Ensure JSON parser allocations are traced (bug#74547)
    
    * src/json.c (json_parser_done):
    (json_make_object_workspace_for_slow_path): Use IGC-aware allocations.

diff --git a/src/json.c b/src/json.c
index eb446f5c221..900fbcbb41a 100644
--- a/src/json.c
+++ b/src/json.c
@@ -807,7 +807,11 @@ json_parser_done (void *parser)
 {
   struct json_parser *p = (struct json_parser *) parser;
   if (p->object_workspace != p->internal_object_workspace)
+#ifdef HAVE_MPS
+    igc_xfree (p->object_workspace);
+#else
     xfree (p->object_workspace);
+#endif
   if (p->byte_workspace != p->internal_byte_workspace)
     xfree (p->byte_workspace);
 }
@@ -833,17 +837,31 @@ json_make_object_workspace_for_slow_path (struct 
json_parser *parser,
   if (parser->object_workspace_size
       == JSON_PARSER_INTERNAL_OBJECT_WORKSPACE_SIZE)
     {
+#ifndef HAVE_MPS
       new_workspace_ptr
        = xnmalloc (new_workspace_size, sizeof (Lisp_Object));
+#else
+      new_workspace_ptr
+       = igc_xalloc_lisp_objs_exact (new_workspace_size);
+#endif
       memcpy (new_workspace_ptr, parser->object_workspace,
              (sizeof (Lisp_Object)
               * parser->object_workspace_current));
     }
   else
     {
+#ifndef HAVE_MPS
       new_workspace_ptr
        = xnrealloc (parser->object_workspace, new_workspace_size,
                     sizeof (Lisp_Object));
+#else
+      new_workspace_ptr
+       = igc_xalloc_lisp_objs_exact (new_workspace_size);
+      memcpy (new_workspace_ptr, parser->object_workspace,
+             (sizeof (Lisp_Object)
+              * parser->object_workspace_current));
+      igc_xfree (parser->object_workspace);
+#endif
     }
 
   parser->object_workspace = new_workspace_ptr;