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.
This commit is contained in:
parent
a70cfadd94
commit
acf38ac064
@ -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.
|
||||
|
||||
@ -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.
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user