bug#71223: 30.0.50; stack overflow after very fast opening and closing of frames

[Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 2101 bytes --]

[ Hi Chong, see below a question for you.  ]

> Could you try the patch below which sequentializes the processing of the
> process-filters to avoid such recursion?

Here's a new version of the patch which fixes various shortcomings
I noticed while trying it out.  Also it shortens the wait times after
displaying a message by applying the `sit-for` principle of not waiting
if there's some "pending input" (where I consider `emacsclient`
processes to be pending inputs as well).  This way, the `sit-for`
waiting times don't accumulate so much.

Also I noticed a weird thing in

    commit 2a847524ab57b1b3d6eaa7e12b96be52dbb79509
    Author: Chong Yidong <cyd@gnu.org>
    Date:   Sat Oct 2 20:03:44 2010 -0400
    
        * lisp/server.el (server-process-filter, server-return-error): Give
        emacsclient time to shut down after receiving an error string.
        
        * etc/NEWS: Document tweak to emacsclient exit status.

    [...]
    diff --git a/lisp/server.el b/lisp/server.el
    index 0f1b0219a2c..e661f055e1a 100644
    --- a/lisp/server.el
    +++ b/lisp/server.el
    @@ -876,6 +876,9 @@ server-process-filter
           (server-log "Authentication failed" proc)
           (server-send-string
            proc (concat "-error " (server-quote-arg "Authentication failed")))
    +      ;; Before calling `delete-process', give emacsclient time to
    +      ;; receive the error string and shut down on its own.
    +      (sit-for 1)
           (delete-process proc)
           ;; We return immediately
           (return-from server-process-filter)))
    @@ -1129,6 +1132,9 @@ server-return-error
          proc (concat "-error " (server-quote-arg
                                  (error-message-string err))))
         (server-log (error-message-string err) proc)
    +    ;; Before calling `delete-process', give emacsclient time to
    +    ;; receive the error string and shut down on its own.
    +    (sit-for 5)
         (delete-process proc)))
     
     (defun server-goto-line-column (line-col)

Why do we wait for 5s in one spot but 1s in another?


        Stefan

[-- Attachment #2: server.patch --]
[-- Type: text/x-diff, Size: 4815 bytes --]

diff --git a/lisp/server.el b/lisp/server.el
index b65053267a6..06bba74bfd7 100644
--- a/lisp/server.el
+++ b/lisp/server.el
@@ -438,7 +438,8 @@ server-sentinel
        (ignore-errors
 	 (delete-file (process-get proc :server-file))))
   (server-log (format "Status changed to %s: %s"
-                      (process-status proc) msg) proc)
+                      (process-status proc) msg)
+              proc)
   (server-delete-client proc))
 
 (defun server--on-display-p (frame display)
@@ -1046,6 +1047,9 @@ server-execute-continuation
     (process-put proc 'continuation nil)
     (if continuation (ignore-errors (funcall continuation)))))
 
+(defvar server--pending-filter-calls nil)
+(defvar server--processing-pending-filters nil)
+
 (cl-defun server-process-filter (proc string)
   "Process a request from the server to edit some files.
 PROC is the server process.  STRING consists of a sequence of
@@ -1145,6 +1149,37 @@ server-process-filter
 `-suspend'
   Suspend this terminal, i.e., stop the client process.
   Sent when the user presses \\[suspend-frame]."
+  ;; Push this to the end of the list, so the list is in the order in which
+  ;; we need to process it.
+  ;; FIXME: This implies an O(N²) worst-case, which is not good:
+  ;; we should use a "true" O(N) queue.
+  (setq server--pending-filter-calls
+        (nconc server--pending-filter-calls (list (cons proc string))))
+  (unless server--processing-pending-filters
+    (server-process-pending-filters)))
+
+(defun server-process-pending-filters ()
+  (let ((server--processing-pending-filters t))
+    (unwind-protect
+        (while server--pending-filter-calls
+          (let* ((oldest (pop server--pending-filter-calls)))
+            (server--process-filter (car oldest) (cdr oldest))))
+      ;; In case we're exiting early (e.g. for `server-goto-toplevel'),
+      ;; make sure we continue running the other pending filters.
+      (when server--pending-filter-calls
+        (run-with-timer 0 nil #'server-process-pending-filters)))))
+
+(defun server--message-timed (time &rest args)
+  (apply #'message args)
+  ;; FIXME: Ideally we should not need `sit-for' here and instead use
+  ;; some `message-timed' call which makes sure the message is visible
+  ;; without forcing us to wait here.
+  ;; If there's already another process-filter pending, skip `sit-for',
+  ;; just as it does when there's pending user input.
+  (unless (consp server--pending-filter-calls)
+    (sit-for time)))
+
+(cl-defun server--process-filter (proc string)
   (server-log (concat "Received " string) proc)
   ;; First things first: let's check the authentication
   (unless (process-get proc :authenticated)
@@ -1158,8 +1193,7 @@ server-process-filter
       ;; Display the error as a message and give the user time to see
       ;; it, in case the error written by emacsclient to stderr is not
       ;; visible for some reason.
-      (message "Authentication failed")
-      (sit-for 2)
+      (server--message-timed 2 "Authentication failed")
       (server-send-string
        proc (concat "-error " (server-quote-arg "Authentication failed")))
       (unless (eq system-type 'windows-nt)
@@ -1169,10 +1203,10 @@ server-process-filter
 	    (delete-terminal terminal))))
       ;; Before calling `delete-process', give emacsclient time to
       ;; receive the error string and shut down on its own.
-      (sit-for 1)
-      (delete-process proc)
+      ;; FIXME: Why do we wait 1s here but 5s in the other one?
+      (run-with-timer 1 nil #'delete-process proc)
       ;; We return immediately.
-      (cl-return-from server-process-filter)))
+      (cl-return-from server--process-filter)))
   (let ((prev (process-get proc 'previous-string)))
     (when prev
       (setq string (concat prev string))
@@ -1507,8 +1541,7 @@ server-return-error
     ;; Display the error as a message and give the user time to see
     ;; it, in case the error written by emacsclient to stderr is not
     ;; visible for some reason.
-    (message (error-message-string err))
-    (sit-for 2)
+    (server--message-timed 2 (error-message-string err))
     (server-send-string
      proc (concat "-error " (server-quote-arg
                              (error-message-string err))))
@@ -1520,8 +1553,8 @@ server-return-error
 	  (delete-terminal terminal))))
     ;; Before calling `delete-process', give emacsclient time to
     ;; receive the error string and shut down on its own.
-    (sit-for 5)
-    (delete-process proc)))
+    ;; FIXME: Why do we wait 5s here but 1s in the other one?
+    (run-with-timer 5 nil #'delete-process proc)))
 
 (defun server-goto-line-column (line-col)
   "Move point to the position indicated in LINE-COL.