ensure nREPL server can exit even when client connections are open (#1003)

Hi,

can you please review patch to let nREPL server to exit even when client
connections are still open. It addresses #1002.

This is simply done by setting the server's `daemon-threads` field to
true.

Additionally, I've updated the `:server*` option accepted by
`server-make!` from an `atom` to a `promise`. This change aligns with
the original intent of providing a server reference when it becomes
available. Note that this is a breaking change as it alters the API.
Given the current development stage and no known users of this API, I
believe it's safe to include this change now. Let me know if you'd
prefer this in a separate PR or think otherwise.

I've added a test for the same.

Thanks

Co-authored-by: ikappaki <[email protected]>

Author: ikappaki

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  * Improved on the nREPL server exception messages by matching that of the REPL user-friendly format (#968)
  * Types created via `deftype` and `reify` may declare supertypes as abstract (taking precedence over true `abc.ABC` types) and specify their member list using `^:abstract-members` metadata (#942)
  * Load functions (`load`, `load-file`, `load-reader`, etc.) now return the value of the last form evaluated. (#984)
+ * nREPL server no longer hangs waiting for client connections to close on exit (#1002)
 
 ### Fixed
  * Fix inconsistent behavior with `basilisp.core/with` when the `body` contains more than one form (#981)

src/basilisp/contrib/nrepl_server.lpy

@@ -433,8 +433,10 @@
   (let [{:keys [host port] :or {host "127.0.0.1" port 0}} opts
         handler (python/type (name (gensym "nREPLTCPHandler"))
                              #py (socketserver/StreamRequestHandler)
-                             #py {"handle" #(on-connect % opts)})]
-    (socketserver/ThreadingTCPServer (python/tuple [host port]) handler)))
+                             #py {"handle" #(on-connect % opts)})
+        server (socketserver/ThreadingTCPServer (python/tuple [host port]) handler)]
+    (set! (.-daemon-threads server) true)
+    server))
 
 (def ^:private nrepl-server-signature
   "The de facto signature nrepl started message that is used by IDEs to identify the
@@ -455,8 +457,8 @@
        to pickup a random available port.
    :keyword ``:nrepl-port-file``: The file to write the port number to, it defaults
        to .nrepl-port.
-   :keyword ``:server*``: An atom to hold the server reference as returned from
-       :lpy:fn:`server-make` , useful for testing.
+   :keyword ``:server*``: A promise object that will receive the server reference
+       from :lpy:fn:`server-make` when is made avaiable.
 
   .. seealso::
 
@@ -467,11 +469,11 @@
    (let [{:keys [nrepl-port-file server*]
           :or   {nrepl-port-file ".nrepl-port"}} opts
          server                                  (server-make opts)]
-     (when server* (reset! server* server))
      (try
        (let [[host port] (py->lisp (.-server-address server))]
          (println (format nrepl-server-signature port host host port))
          (spit nrepl-port-file (str port)))
+       (when server* (deliver server* server))
        (.serve-forever server)
        (catch python/KeyboardInterrupt _e
          (println "Exiting in response to a keyboard interrupt..."))

tests/basilisp/contrib/nrepl_server_test.lpy

@@ -605,26 +605,48 @@
                    (throw python/ZeroDivisionError)))))
 
   (testing "nrepl-server port file and address"
-    (let [server* (atom nil)
+    (let [server* (promise)
           [fd filename] (tempfile/mkstemp "nrepl-server-port-test")]
       (doto (threading/Thread
              **
              :target #(nr/start-server! {:server* server* :nrepl-port-file filename :host "0.0.0.0"})
              :daemon true)
         (.start))
-      (try
-        (time/sleep 1) ;; give some time to the server to settle down
-        (is @server*)
-        (is (bio/exists? filename))
-        (let [port-filename  (slurp filename)
-              server @server*
-              [host port] (py->lisp (.-server-address server))]
-          (is (= host "0.0.0.0"))
-          (is (= (str port) port-filename)))
-
-        (finally
-          (doto @server*
-            (.shutdown)
-            (.server-close))
-          (os/close fd)
-          (os/unlink filename))))))
+      (let [server (deref server* 1 nil)]
+        (is server)
+        (try
+          (is (bio/exists? filename))
+          (let [port-filename  (slurp filename)
+                [host port] (py->lisp (.-server-address server))]
+            (is (= host "0.0.0.0"))
+            (is (= (str port) port-filename)))
+
+          (finally
+            (doto server
+              (.shutdown)
+              (.server-close))
+            (os/close fd)
+            (os/unlink filename)))))))
+
+(deftest nrepl-server-shutdown-test
+  (testing "server can exit even when client connections are still open."
+    (let [server* (promise)
+          server-thread (threading/Thread
+                         **
+                         :target #(nr/start-server! {:server* server*})
+                         :daemon true)]
+      (.start server-thread)
+      (let [server (deref server* 1 nil)]
+        (is server)
+        (let [[host_ port] (py->lisp (.-server-address server))]
+          (binding [*nrepl-port* port]
+            (with-connect client
+              (client-send! client {:id 1 :op "clone"})
+              (let [{:keys [status]} (client-recv! client)]
+                (is (= ["done"] status)))
+              ;; Shutdown server during an active client connection
+              (let [shutdown-thread (future (doto server
+                                              (.server-close))
+                                            :done)
+                    status (deref shutdown-thread 1 :time-out)]
+                (is (= :done status))))))))))