From 8f50231695d4a28a7d0caba7ad5bef04afe4b729 Mon Sep 17 00:00:00 2001 From: Kinneyzhang Date: Mon, 27 Jul 2026 01:56:59 +0800 Subject: [PATCH] Clear the pending queue on reset and drain it under unwind-protect tp-reactive-reset cleared the four reactive registries and the buffer registry but not the global tp--batch-update-pending queue. Entries queued there by a nested variable write during a non-batched update were flushed by a tail that sat OUTSIDE any unwind-protect, so an error escaping the re-render (for example from a modification hook) stranded them; a later tp-layer-reset left the ghost entries naming layers that no longer existed, and the next completed update's flush replayed them against the fresh registry. Fix: - tp-reactive-reset now also clears tp--batch-update-pending (and so does tp-layer-reset, which delegates to it). - tp--reactive-apply-update wraps the non-nested update body in unwind-protect with the existing flush tail as the cleanup form, so nested queue entries flush even when the re-render signals; the reentrancy guard is unbound by then, so the cleanup flush re-renders exactly like the success path. Tests port the review's probes: a queued ghost entry disappears on tp-reactive-reset, and a before-change-functions hook that writes a second reactive variable and then signals leaves the queue drained with the nested variable's re-render applied. Co-Authored-By: Claude Fable 5 --- tp-reactive.el | 4 +++ tp-render-tests.el | 58 +++++++++++++++++++++++++++++++++ tp-render.el | 80 +++++++++++++++++++++++++--------------------- 3 files changed, 105 insertions(+), 37 deletions(-) diff --git a/tp-reactive.el b/tp-reactive.el index b4e94cf..7ff336c 100644 --- a/tp-reactive.el +++ b/tp-reactive.el @@ -483,6 +483,10 @@ it to allow re-definition to change initial values." (setq tp-layer-watchers nil) (setq tp-layer-computed nil) (setq tp-layer-data nil) + ;; Drop queued re-renders too: entries stranded by an error escaping + ;; an update would otherwise survive the reset and replay against + ;; freshly (re)defined layers on the next flush (ARCH-4). + (setq tp--batch-update-pending nil) (clrhash tp--layer-buffers)) (provide 'tp-reactive) diff --git a/tp-render-tests.el b/tp-render-tests.el index 4c6e4c3..210b3d6 100644 --- a/tp-render-tests.el +++ b/tp-render-tests.el @@ -661,6 +661,64 @@ inside the replacement span." "emacs")) (should (eq (get-text-property 1 'face) 'bold))))) +;;; ARCH-4: the pending queue must survive neither reset nor errors + +(defvar tp-rt-a4-face nil) +(defvar tp-rt-a4-color nil) + +(ert-deftest tp-render-test-reactive-reset-clears-pending-queue () + "tp-reactive-reset drops queued batch re-renders (ARCH-4). +Stranded entries would otherwise survive the reset and replay against +freshly (re)defined layers on the next flush." + (unwind-protect + (progn + (tp--queue-batch-update 'tp-rt-a4-ghost 'tp-rt-a4-ghost-var nil nil) + (should tp--batch-update-pending) + (tp-reactive-reset) + (should (null tp--batch-update-pending))) + (setq tp--batch-update-pending nil))) + +(ert-deftest tp-render-test-error-escaping-update-flushes-nested-queue () + "An error escaping a re-render cannot strand nested queued updates. +A modification hook that writes a second reactive variable and then +signals used to strand the nested entry in the global queue - the +flush tail sat outside any unwind-protect. The flush now runs as the +update unwinds, so the nested variable's re-render still lands and +the queue is drained (ARCH-4)." + (setq tp-rt-a4-face 'bold + tp-rt-a4-color "red") + (unwind-protect + (progn + (define-tp tp-rt-a4-layer-a () '(face $tp-rt-a4-face)) + (define-tp tp-rt-a4-layer-b () + '(face (:foreground $tp-rt-a4-color))) + (with-temp-buffer + (insert "Hello world") + (tp-set 1 6 'tp-rt-a4-layer-a) + (tp-set 7 12 'tp-rt-a4-layer-b) + (let ((armed t)) + (add-hook 'before-change-functions + (lambda (_beg _end) + (when armed + (setq armed nil) + ;; Nested reactive write from within the + ;; re-render: goes to the global queue. + (setq tp-rt-a4-color "green") + (error "boom from modification hook"))) + nil t) + (should-error (setq tp-rt-a4-face 'italic)) + ;; The nested entry was flushed on the way out, not + ;; stranded... + (should (null tp--batch-update-pending)) + ;; ...and its re-render landed despite the error. + (should (equal (get-text-property 7 'face) + '(:foreground "green")))))) + (tp-undefine-layer 'tp-rt-a4-layer-a) + (tp-undefine-layer 'tp-rt-a4-layer-b) + (setq tp-rt-a4-face nil + tp-rt-a4-color nil + tp--batch-update-pending nil))) + ;;; R3 (0.3.0): anonymous-layer garbage collection (ert-deftest tp-render-test-gc-collects-unreferenced-anonymous-layer () diff --git a/tp-render.el b/tp-render.el index a88d45a..d01e2f7 100644 --- a/tp-render.el +++ b/tp-render.el @@ -668,43 +668,49 @@ installed as `tp--reactive-update-function'." (if tp--reactive-updating ;; Nested change fired from within an update: queue, don't recurse. (tp--queue-batch-update layer-name symbol where tp-text-affected) - (let ((tp--reactive-updating t)) - ;; Update computed properties for this layer - (let ((updated-override - (tp--update-layer-computed layer-name override-alist))) - ;; Update only the reactive properties in the layer definition. - ;; Buffer-local changes must not leak into the global definition; - ;; the buffer re-render below resolves against the buffer-local - ;; values instead. - (when (and reactive-props (not (bufferp where))) - (let ((resolved-props (tp--resolve-reactive-symbols - reactive-props updated-override)) - (current-props (cdr (assoc layer-name tp-layer-alist)))) - (when current-props - ;; Deep merge the resolved reactive props into the current - ;; layer props to preserve nested plist values (like face) - (tp--set-layer-props - layer-name - (tp--deep-merge-plist current-props resolved-props))))) - ;; Update text regions with this layer (or defer if batching) - (if tp--batch-update-active - ;; Batching: defer the buffer update - (progn - (tp-debug-log " Deferring buffer update for %s (batch mode)" - layer-name) - (tp--queue-batch-update layer-name symbol where - tp-text-affected)) - ;; Normal: update immediately - (tp-debug-log " Updating layer %s (tp-text affected: %s)" - layer-name (if tp-text-affected "yes" "no")) - (if tp-text-affected - (tp--update-reactive-text layer-name where updated-override) - (tp--update-layer-regions layer-name where updated-override))))) - ;; Re-renders queued by nested variable writes during this update are - ;; flushed now that the outermost update has finished. - (unless tp--batch-update-active - (when tp--batch-update-pending - (tp--flush-batch-updates)))))) + (unwind-protect + (let ((tp--reactive-updating t)) + ;; Update computed properties for this layer + (let ((updated-override + (tp--update-layer-computed layer-name override-alist))) + ;; Update only the reactive properties in the layer definition. + ;; Buffer-local changes must not leak into the global definition; + ;; the buffer re-render below resolves against the buffer-local + ;; values instead. + (when (and reactive-props (not (bufferp where))) + (let ((resolved-props (tp--resolve-reactive-symbols + reactive-props updated-override)) + (current-props (cdr (assoc layer-name tp-layer-alist)))) + (when current-props + ;; Deep merge the resolved reactive props into the current + ;; layer props to preserve nested plist values (like face) + (tp--set-layer-props + layer-name + (tp--deep-merge-plist current-props resolved-props))))) + ;; Update text regions with this layer (or defer if batching) + (if tp--batch-update-active + ;; Batching: defer the buffer update + (progn + (tp-debug-log " Deferring buffer update for %s (batch mode)" + layer-name) + (tp--queue-batch-update layer-name symbol where + tp-text-affected)) + ;; Normal: update immediately + (tp-debug-log " Updating layer %s (tp-text affected: %s)" + layer-name (if tp-text-affected "yes" "no")) + (if tp-text-affected + (tp--update-reactive-text layer-name where updated-override) + (tp--update-layer-regions layer-name where updated-override))))) + ;; Re-renders queued by nested variable writes during this update + ;; are flushed now that the outermost update has finished. The + ;; flush runs under unwind-protect so an error escaping the + ;; re-render (for example from a modification hook) cannot strand + ;; queued entries in the global queue (ARCH-4); the reentrancy + ;; guard has been unbound by now, so the flush re-renders + ;; normally. + (unless tp--batch-update-active + (when tp--batch-update-pending + (tp--flush-batch-updates))))))) (defun tp--reactive-flush-entry (layer-name where tp-text-affected) "Re-render LAYER-NAME's regions in WHERE (or all buffers when nil).