Revise command process connection

Goal: Fix any remnants of issue #344.

- Simplify code paths by relying on sentinels to do cleanup
  when the process is deleted or closed.

- Analyze the remaining paths to ensure that racket--cmd-connecting-p
  is set t during a series of connection attempts, and, set back to
  nil after such a series has completed (whether succeeded, failed, or
  C-g by the user).

- Break out the guts of racket--cmd-connect-start to a helper
  racket--cmd-connect-attempt. This makes it clearer that the former
  does checks needed at the start of the series, and the latter is
  called at intervals via run-at-timer to "run in the background".

- Rather than setting racket--cmd-proc back to nil, leave it set to
  the last value, and use things like `process-status` determine if it
  is 'open. Provide a little helper for this: racket--cmd-open-p.

- Add more (and hopefully better) comments and doc strings.

Author: Greg Hendershott

racket-repl.el

@@ -1,6 +1,6 @@
 ;;; racket-repl.el -*- lexical-binding: t; -*-
 
-;; Copyright (c) 2013-2018 by Greg Hendershott.
+;; Copyright (c) 2013-2019 by Greg Hendershott.
 ;; Portions Copyright (C) 1985-1986, 1999-2013 Free Software Foundation, Inc.
 ;; Image portions Copyright (C) 2012 Jose Antonio Ortega Ruiz.
 
@@ -258,15 +258,9 @@ used. See the latter for more information."
           (when display
             (display-buffer (current-buffer)))
           (message "Starting %s to run %s ..." racket-program run.rkt)
-          ;; Ensure command server connection closed when racket process dies.
-          (set-process-sentinel proc
-                                (lambda (_proc event)
-                                  (with-racket-repl-buffer
-                                    (insert (concat "Process Racket REPL " event)))
-                                  (racket--cmd-disconnect)))
           (set-process-coding-system proc 'utf-8 'utf-8) ;for e.g. λ
           (racket-repl-mode)))))
-  (unless (racket--cmd-connected-or-connecting-p)
+  (unless (racket--cmd-open-or-connecting-p)
     (racket--cmd-connect-start)))
 
 (defun racket--version ()
@@ -292,10 +286,15 @@ used. See the latter for more information."
 ;;; Connection to command process
 
 (defvar racket--cmd-proc nil
-  "Process when connection to the command server is established.")
-(defvar racket--cmd-buf nil
-  "Process buffer when connection to the command server is established.")
-(defvar racket--cmd-connecting-p nil)
+  "Process for talking to the command server.
+Use `racket--cmd-open-p' to check if it is non-nil and in an
+'open state.")
+
+(defvar racket--cmd-connecting-p nil
+  "True while a series of connection attempts is underway.
+See `racket--cmd-connect-start' and`racket--cmd-connect-finish'.
+Used to guard against starting more than one series of connection
+attempts at once.")
 
 (defvar racket--cmd-nonce->callback (make-hash-table :test 'eq)
   "A hash from nonce to callback function.")
@@ -305,76 +304,90 @@ used. See the latter for more information."
 (defvar racket--cmd-connect-attempts 15)
 (defvar racket--cmd-connect-timeout 15)
 
-(defun racket--cmd-connected-or-connecting-p ()
-  (and (or racket--cmd-proc racket--cmd-connecting-p) t))
-
-(defun racket--cmd-connect-start (&optional attempt)
-  "Start to connect to the Racket command process.
-
-If already connected, disconnects first.
-
-The command server may might not be ready to accept connections,
-because Racket itself and our backend are still starting up.
-After calling this, call `racket--cmd-connect-finish' to
-wait for the connection to be established."
+(defun racket--cmd-open-p ()
+  (and racket--cmd-proc
+       (eq 'open (process-status racket--cmd-proc))))
+
+(defun racket--cmd-open-or-connecting-p ()
+  (or (racket--cmd-open-p)
+      racket--cmd-connecting-p))
+
+(defun racket--cmd-connect-start ()
+  "Start a series of attempts to connect to the Racket command process.
+
+The command server might not be ready to accept connections,
+because Racket itself and our backend are still starting up. We
+don't want to block Emacs while waiting. Instead: Call this to
+start a series of connection attempts that will \"happen in the
+background\" at intervals using `run-with-timer'. After this
+returns, then later when eventually something really must wait
+for the connection, to issue a command, call
+`racket--cmd-connect-finish'"
   (unless (featurep 'make-network-process '(:nowait t))
     (error "racket-mode needs Emacs to support the :nowait feature"))
-  (let ((attempt (or attempt 1)))
-    (when (= attempt 1)
-      (racket--cmd-disconnect)
-      (setq racket--cmd-connecting-p t))
-    (make-network-process
-     :name    "racket-command"
-     :host    "127.0.0.1"
-     :service racket-command-port
-     :nowait  t
-     :sentinel
-     (lambda (proc event)
-       ;;(message "sentinel process %S event %S attempt %s" proc event attempt)
-       (cond
-        ((string-match-p "^open" event)
-         (setq racket--cmd-proc proc)
-         (setq racket--cmd-buf (generate-new-buffer
-                                (concat " " (process-name proc))))
-         (buffer-disable-undo racket--cmd-buf)
-         (set-process-filter proc #'racket--cmd-process-filter)
-         (process-send-string proc (concat racket--cmd-auth "\n"))
-         (message "Connected to %s process on port %s after %s attempt(s)"
-                  proc racket-command-port attempt))
-        ((string-match-p "^failed" event)
-         (delete-process proc)
-         (when (<= attempt racket--cmd-connect-attempts)
-           (run-at-time 1.0 nil
-                        #'racket--cmd-connect-start
-                        (1+ attempt))))
-        (t (racket--cmd-disconnect)))))))
+  (when (racket--cmd-open-p)
+    (error "racket--cmd-proc already open"))
+  (when racket--cmd-connecting-p
+    (error "already in a series attempts to connect to racket command process"))
+  (setq racket--cmd-connecting-p t)
+  (racket--cmd-connect-attempt 1))
+
+(defun racket--cmd-connect-attempt (attempt)
+  "Only for use by `racket--cmd-connect-start'."
+  (setq
+   racket--cmd-proc
+   (make-network-process
+    :name    "racket-command"
+    :host    "127.0.0.1"
+    :service racket-command-port
+    :nowait  t
+    :sentinel
+    (lambda (proc event)
+      ;;(message "sentinel got (%S %S) [attempt %s]" proc event attempt)
+      (cond ((string-match-p "^open" event)
+             (let ((buf (generate-new-buffer (concat "*" (process-name proc) "*"))))
+               (set-process-buffer proc buf)
+               (buffer-disable-undo buf))
+             (set-process-filter proc #'racket--cmd-process-filter)
+             (process-send-string proc (concat racket--cmd-auth "\n"))
+             (setq racket--cmd-connecting-p nil)
+             (message "Connected to %s process on port %s after %s attempt(s)"
+                      proc racket-command-port attempt))
+
+            ((string-match-p "^failed" event)
+             (delete-process proc) ;we'll get called with "deleted" event, below
+             (if (<= attempt racket--cmd-connect-attempts)
+                 (run-at-time 1.0 nil
+                              #'racket--cmd-connect-attempt
+                              (1+ attempt))
+               (setq racket--cmd-connecting-p nil)))
+
+            ((or (string-match-p "^deleted" event)
+                 (string-match-p "^connection broken by remote peer" event))
+             (clrhash racket--cmd-nonce->callback))
+
+            (t (message "sentinel surprised by (%S %S) [attempt %s]" proc event attempt)))))))
 
 (defun racket--cmd-connect-finish ()
+  "Call this to wait for the series of connection attempts to complete."
+  ;; Important: Every flow path must set `racket--cmd-connecting-p'
+  ;; back to nil. For example: 1. Do so before calling `error'. 2. Use
+  ;; `inhibit-quit' to handle the user getting tired of waiting and
+  ;; pressing C-g.
   (with-timeout (racket--cmd-connect-timeout
                  (setq racket--cmd-connecting-p nil)
                  (error "Could not connect to racket-command process on port %s"
                         racket-command-port))
-    (while (not racket--cmd-proc)
-      (message "Still trying to connect to racket-command process on port %s ..."
-               racket-command-port)
-      (sit-for 0.2))
-    (setq racket--cmd-connecting-p nil)))
-
-(defun racket--cmd-disconnect ()
-  "Disconnect from the Racket command process."
-  (setq racket--cmd-connecting-p nil)
-  (when racket--cmd-proc
-    ;; Sentinel calls us for "deleted" event, which we ourselves will
-    ;; trigger with the `delete-process' below. So set
-    ;; racket--cmd-proc nil before calling `delete-process'.
-    (let ((proc (prog1 racket--cmd-proc (setq racket--cmd-proc nil)))
-          (buf  (prog1 racket--cmd-buf  (setq racket--cmd-buf nil))))
-      (delete-process proc)
-      (kill-buffer buf)
-      (clrhash racket--cmd-nonce->callback))))
-
-(defun racket--cmd-process-filter (_proc string)
-  (let ((buffer racket--cmd-buf))
+    (let ((inhibit-quit t))
+      (while (and (not quit-flag)
+                  (not (racket--cmd-open-p)))
+        (message "Still trying to connect to racket-command process on port %s ..."
+                 racket-command-port)
+        (sit-for 0.2))
+      (setq racket--cmd-connecting-p nil))))
+
+(defun racket--cmd-process-filter (proc string)
+  (let ((buffer (process-buffer proc)))
     (when (buffer-live-p buffer)
       (with-current-buffer buffer
         (goto-char (point-max))
@@ -425,6 +438,8 @@ form."
   (when (and callback
              (not (equal callback #'ignore)))
     (puthash racket--cmd-nonce callback racket--cmd-nonce->callback))
+  (unless (racket--cmd-open-p)
+    (error "racket command process not open"))
   (process-send-string racket--cmd-proc
                        (format "%S\n" (cons racket--cmd-nonce
                                             command-sexpr))))