[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#58639: 29.0.50; [noverlay] Nested overlay iteration in GC
From: |
Matt Armstrong |
Subject: |
bug#58639: 29.0.50; [noverlay] Nested overlay iteration in GC |
Date: |
Wed, 19 Oct 2022 15:16:53 -0700 |
Sorry Stefan, that last patch won't build against the current
feature/noverlay. I sent it from a tree that was not fully merged.
This one renames the itree_busy_p call to itree_iterator_busy_p.
>From 7bd90841b07a58b16c08b8bc07b94b0946aca1f3 Mon Sep 17 00:00:00 2001
From: Matt Armstrong <matt@rfc20.org>
Date: Wed, 19 Oct 2022 13:42:35 -0700
Subject: [PATCH] Revert "mark_overlays: Use the normal ITREE_FOREACH"
This reverts commit b8fbd42f0a7caa4cd9e2d50dd4e4b2101ac78acd,
with edits.
* src/alloc.c (mark_overlays): restore function.
(mark_buffer): Call it, not ITREE_FOREACH.
(garbage_collect): eassert (!itree_busy_p ()).
* src/itree.h: Comment tweak: explain why GC is considered risky. It
isn't that GC itself is risky, it is that GC can call ELisp by way of
a hook, and running ELisp during iteration is risks nested iteration.
---
src/alloc.c | 22 +++++++++++++++++++---
src/itree.h | 3 ++-
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/src/alloc.c b/src/alloc.c
index 00f2991f250..d7e0a99ffe7 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -6279,6 +6279,11 @@ garbage_collect (void)
image_prune_animation_caches (false);
#endif
+ /* ELisp code run by `gc-post-hook' could result in itree iteration,
+ which must not happen while the itree is already busy. See
+ bug#58639. */
+ eassert (!itree_iterator_busy_p ());
+
if (!NILP (Vpost_gc_hook))
{
specpdl_ref gc_count = inhibit_garbage_collection ();
@@ -6510,6 +6515,18 @@ mark_overlay (struct Lisp_Overlay *ov)
mark_object (ov->plist);
}
+/* Mark the overlay subtree rooted at NODE. */
+
+static void
+mark_overlays (struct interval_node *node)
+{
+ if (node == NULL)
+ return;
+ mark_object (node->data);
+ mark_overlays (node->left);
+ mark_overlays (node->right);
+}
+
/* Mark Lisp_Objects and special pointers in BUFFER. */
static void
@@ -6531,9 +6548,8 @@ mark_buffer (struct buffer *buffer)
if (!BUFFER_LIVE_P (buffer))
mark_object (BVAR (buffer, undo_list));
- struct interval_node *node;
- ITREE_FOREACH (node, buffer->overlays, PTRDIFF_MIN, PTRDIFF_MAX, ASCENDING)
- mark_object (node->data);
+ if (buffer->overlays)
+ mark_overlays (buffer->overlays->root);
/* If this is an indirect buffer, mark its base buffer. */
if (buffer->base_buffer &&
diff --git a/src/itree.h b/src/itree.h
index f98f028ea52..8647168c935 100644
--- a/src/itree.h
+++ b/src/itree.h
@@ -152,7 +152,8 @@ itree_iterator_start (struct interval_tree *tree, ptrdiff_t
begin,
it is cheap a pure.
- Only a single iteration can happen at a time, so make sure none of the
code within the loop can start another tree iteration, i.e. it shouldn't
- be able to run ELisp code (or GC for that matter).
+ be able to run ELisp code, nor GC since GC can run ELisp by way
+ of `post-gc-hook`.
- If you need to exit the loop early, you *have* to call `ITREE_ABORT`
just before exiting (e.g. with `break` or `return`).
- Non-local exits are not supported within the body of the loop.
--
2.35.1