From acf38ac064413a2e9d5633fb185e5ac7d412fe6b Mon Sep 17 00:00:00 2001 From: Joseph Ferano Date: Fri, 18 Sep 2026 07:30:43 +0700 Subject: [PATCH] The poll settles like every other sender, and the settle runs before the reconnect `flan-dev--poll' guarded on `flan-dev--busy' and nothing else, and the watch timer is precisely the sender that does not bind it: it sends without waiting, so it has nothing to hold. A poll landing in that gap sent `describe' down a connection that already owed a reply, read the watch's answer as its own, and left its own for the watch to read a tick later -- after which the two consumers stayed swapped for the session, each of them answering the other's question. It runs `flan-dev-settle-hook' now, which is the invariant the hook's docstring already claimed. And the hook runs before `flan-dev--live-connection' rather than after. An outstanding reply belongs to the connection it was asked on; when a daemon has been restarted the old connection is gone and no reply is coming on the new one, but the hook was being asked about the new one and blocked a full `flan-dev-reply-timeout' -- a frozen Emacs on the first key pressed after a restart, with the watch armed. `flan-dev--busy' now covers the reconnect too, which asks the new daemon what it defines and so must not be interrupted by a poll either. --- emacs/flan-dev.el | 34 ++++++++++++++++++++++++++++------ emacs/test-flan-dev.el | 31 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 6 deletions(-) diff --git a/emacs/flan-dev.el b/emacs/flan-dev.el index 3f39b38..ac512e1 100644 --- a/emacs/flan-dev.el +++ b/emacs/flan-dev.el @@ -388,22 +388,35 @@ with it, and a rejected evaluation is a likely moment to *become* stopped." reply) (defvar flan-dev-settle-hook nil - "Run before a request is sent, with the connection already open. + "Run before anything is sent, against the connection as it stands. The protocol is one reply per request on one connection, and that is the whole reason this exists. Anything that sends without waiting — the watch timer is the only such thing — leaves a reply in flight that the *next* request would otherwise read as its own. So a sender-in-flight hangs a function here that collects its own reply first, and the invariant holds: exactly one request -outstanding, and every reply consumed by whoever asked for it.") +outstanding, and every reply consumed by whoever asked for it. + +Before the connection is checked, not after, and that ordering is the point. +An outstanding reply belongs to the connection it was asked on; if the daemon +has been restarted under Emacs, that connection is gone and no reply is coming +on the new one. Running this first is what lets a hook see that for itself +and drop its pending flag, rather than sitting out a full +`flan-dev-reply-timeout' waiting on a socket the question was never asked +down.") (defun flan-dev--request (form) "Send FORM to the connected program and return its reply." - (let* ((proc (flan-dev--live-connection)) - (flan-dev--busy t)) + ;; `flan-dev--busy' first of all, and around the reconnect as well as around + ;; the send: `flan-dev--live-connection' asks the new daemon what it + ;; defines, and `accept-process-output' runs timers, so a poll firing in the + ;; middle of that would be a second conversation on the connection this one + ;; just opened. + (let ((flan-dev--busy t)) (run-hooks 'flan-dev-settle-hook) - (flan-dev--absorb (progn (flan-dev--send proc form) - (flan-dev--read-reply proc))))) + (let ((proc (flan-dev--live-connection))) + (flan-dev--absorb (progn (flan-dev--send proc form) + (flan-dev--read-reply proc)))))) ;;; Noticing that the program stopped @@ -441,6 +454,15 @@ Set to nil to leave the program's state to whatever replies happen to say." (let ((proc flan-dev--connection) (flan-dev--busy t)) (ignore-errors + ;; The same settle every other sender does, and for the same reason. + ;; `flan-dev--busy' is not enough on its own: the watch timer leaves a + ;; request in flight and *clears* nothing, deliberately — it binds no + ;; busy flag, because it never waits — so a poll that checked only the + ;; flag would send `describe' with the watch's reply still coming and + ;; read that instead. The two would then stay swapped for the rest of + ;; the session, each consumer answering the other's question, which is + ;; exactly the interleaving `flan-dev-settle-hook' exists to prevent. + (run-hooks 'flan-dev-settle-hook) ;; `describe' rather than `break': it is the cheap op, it is what ;; drains the program's output, and the state is on every reply anyway. ;; Asking `break' would fetch restart names nobody is choosing from. diff --git a/emacs/test-flan-dev.el b/emacs/test-flan-dev.el index 5860e23..b29db9b 100644 --- a/emacs/test-flan-dev.el +++ b/emacs/test-flan-dev.el @@ -743,6 +743,37 @@ is written instead — the real `message' call the real command makes." (flan-watch--tick) (test-flan--check "a tick leaves a request in flight rather than waiting for it" flan-watch--pending) + ;; The background poll is a sender too, and it was the one sender that did + ;; not settle: it guarded on `flan-dev--busy' alone, which the watch timer + ;; deliberately does not bind — it never waits, so it has nothing to hold — + ;; and sent `describe' straight into a connection that already owed a reply. + ;; It then read the watch's answer as its own, and the two stayed swapped + ;; for the rest of the session. The second check is where that would show: + ;; a `describe' answered by the watch table has no `:fns' in it at all. + (flan-dev--poll) + (test-flan--check "a poll settles the watch's reply rather than reading it as its own" + (null flan-watch--pending)) + (test-flan--check "and the request after it is still answered by its own reply" + (member "step" (plist-get (flan-dev--request '(:op "describe")) + :fns))) + ;; And the same hook against a daemon restarted under an armed watch. The + ;; reply the watch is owed was asked for on the connection that has gone, so + ;; there is nothing to wait for — running the hook before the connection is + ;; checked is what lets it see that. Asking after the reconnect meant a + ;; whole `flan-dev-reply-timeout' of frozen Emacs on the first thing anybody + ;; typed after a restart, which is why the wait itself is what is measured. + (flan-watch--tick) + (delete-process flan-dev--connection) + (let ((flan-dev-reply-timeout 2) + (started (float-time))) + (let ((r (flan-dev--request '(:op "describe")))) + (test-flan--check "a request after a restart does not wait out a reply the old connection owed" + (and (member "step" (plist-get r :fns)) + (< (- (float-time) started) 2))) + (test-flan--check "and the watch is not left waiting for one either" + (null flan-watch--pending)))) + ;; A request in flight again, for the interleaving below. + (flan-watch--tick) ;; And now the interleaving, with a reply outstanding on purpose. If the ;; settle hook were not there this would return the watch table's plist and ;; `flan-dev--report' would take its missing :status for a rejection.