Improves the way ns processes are spawn. Dont need to use threads anymore. (#234)

Author: jmgomez

nimlangserver.nim

@@ -1,6 +1,6 @@
 import json_rpc/[servers/socketserver, private/jrpc_sys, jsonmarshal, rpcclient, router]
 import chronicles, chronos
-import std/[ syncio, os, json, strutils, strformat, net]
+import std/[ syncio, os, json, strutils, strformat]
 import ls, routes, suggestapi, utils, lstransports, asyncprocmonitor
 import protocol/types
 when defined(posix):
@@ -45,13 +45,6 @@ proc registerRoutes(srv: RpcSocketServer, ls: LanguageServer) =
   srv.register("textDocument/didChange", wrapRpc(partial(didChange, ls)))
   srv.register("$/setTrace", wrapRpc(partial(setTrace, ls)))
 
-proc getNextFreePort*(): Port= 
-  let s = newSocket()
-  s.bindAddr(Port(0), "localhost")
-  let (_, port) = s.getLocalAddr
-  s.close()
-  port
-  
 proc handleParams(): CommandLineParams =
   if paramCount() > 0 and paramStr(1) in ["-v", "--version"]:
     echo LSPVersion

nimlangserver.nimble

@@ -14,7 +14,8 @@ requires "nim == 2.0.8",
          "with",
          "chronicles",
          "serialization",
-         "json_serialization"
+         "json_serialization",
+         "stew"
 
 
 --path:"."

suggestapi.nim

@@ -12,7 +12,9 @@ import osproc,
   chronicles,
   protocol/types,
   std/options,
-  chronos
+  chronos,
+  chronos/asyncproc,
+  stew/[byteutils]
 
 const REQUEST_TIMEOUT* = 120000
 const HighestSupportedNimSuggestProtocolVersion = 4
@@ -78,7 +80,7 @@ type
     openFiles*: OrderedSet[string]
     successfullCall*: bool
     errorCallback: NimsuggestCallback
-    process*: Process
+    process*: AsyncProcessRef
     port*: int
     root: string
     requestQueue: Deque[SuggestCall]
@@ -224,30 +226,11 @@ proc markFailed(self: Nimsuggest, errMessage: string) {.raises: [].} =
   if self.errorCallback != nil:
     self.errorCallback(self)
 
-proc readPort(param: tuple[ns: ptr NimSuggestImpl, process: Process]) {.thread.} =
-  try:
-    var line = param.process.outputStream.readLine & "\n"
-    param.ns.port = line.strip.parseInt
-  except CatchableError:
-    error "Failed to read nimsuggest port"
-    var msg = failedToken & "\n"
-    param.ns.port = -1
-
-proc logStderr(param: tuple[root: string, process: Process]) {.thread.} =
-  try:
-    var line = param.process.errorStream.readLine
-    while line != "\0":
-      stderr.writeLine &"******NIM SUGGEST ERRROR***** {param.root} \n"
-      stderr.writeLine fmt ">> {line}"
-      line = param.process.errorStream.readLine
-  except IOError:
-    discard
-
 proc stop*(self: Nimsuggest) =
   debug "Stopping nimsuggest for ", root = self.root
   try:
-    self.process.kill()
-    self.process.close()
+    let res = self.process.kill()
+    debug "Stopped nimsuggest ", res = res
   except Exception as ex:
     writeStackTrace(ex)
 
@@ -306,14 +289,10 @@ proc getNimsuggestCapabilities*(nimsuggestPath: string):
       if cap.isSome:
         result.incl(cap.get)
 
-proc waitUntilPortIsRead(ns: NimSuggest): Future[void] {.async.} =
-  while ns.port == 0:
-    await sleepAsync(10)
-
-  if ns.port == -1:
-    ns.markFailed "Failed to start nimsuggest"
-  else:
-    debug "Started nimsuggest", port = ns.port, root = ns.root
+proc logNsError(ns: NimSuggest) {.async.} = 
+  let err = string.fromBytes(ns.process.stderrStream.read().await)
+  error "NimSuggest Error (stderr)", err = err
+  ns.markFailed(err)
 
 proc createNimsuggest*(root: string,
                        nimsuggestPath: string,
@@ -324,13 +303,6 @@ proc createNimsuggest*(root: string,
                        workingDir = getCurrentDir(),
                        enableLog: bool = false,
                        enableExceptionInlayHints: bool = false): Future[Nimsuggest] {.async, gcsafe.} =
-  var
-    thread: Thread[tuple[ns: ptr NimSuggestImpl, process: Process]]
-    stderrThread: Thread[tuple[root: string, process: Process]]
-
-  info "Starting nimsuggest", root = root, timeout = timeout, path = nimsuggestPath, 
-    workingDir = workingDir
-
   result = Nimsuggest()
   result.requestQueue = Deque[SuggestCall]()
   result.root = root
@@ -340,11 +312,14 @@ proc createNimsuggest*(root: string,
   result.nimSuggestPath = nimsuggestPath
   result.version = version
 
+  info "Starting nimsuggest", root = root, timeout = timeout, path = nimsuggestPath, 
+    workingDir = workingDir
+
   if nimsuggestPath != "":
     result.protocolVersion = detectNimsuggestVersion(root, nimsuggestPath, workingDir)
     if result.protocolVersion > HighestSupportedNimSuggestProtocolVersion:
       result.protocolVersion = HighestSupportedNimSuggestProtocolVersion
-    var
+    var      
       args = @[root, "--v" & $result.protocolVersion, "--autobind"]
     if result.protocolVersion >= 4:
       args.add("--clientProcessId:" & $getCurrentProcessId())
@@ -357,18 +332,13 @@ proc createNimsuggest*(root: string,
         args.add("--exceptionInlayHints:on")
       else:
         args.add("--exceptionInlayHints:off")
-    result.process = startProcess(command = nimsuggestPath,
-                                  workingDir = workingDir,
-                                  args = args,
-                                  options = {poUsePath})
-    #TODO better use startProcess from chronos so we dont need to spawn a thread
-    # all this is needed to avoid the need to block on the main thread.
-    createThread(thread, readPort, (ns: cast[ptr NimSuggestImpl](result), process: result.process))
-
-    # copy stderr of log
-    createThread(stderrThread, logStderr, (root: root, process: result.process))
-    await waitUntilPortIsRead(result)   
-
+    result.process =
+      await startProcess(nimsuggestPath, arguments = args, options = { UsePath }, 
+            stdoutHandle = AsyncProcess.Pipe,
+            stderrHandle = AsyncProcess.Pipe)
+            
+    asyncSpawn logNsError(result)
+    result.port = (await result.process.stdoutStream.readLine(sep="\n")).parseInt 
   else:
     error "Unable to start nimsuggest. Unable to find binary on the $PATH", nimsuggestPath = nimsuggestPath
     result.markFailed fmt "Unable to start nimsuggest. `{nimsuggestPath}` is not present on the PATH"

tests/textensions.nim

@@ -2,10 +2,11 @@ import ../[
   nimlangserver, ls, lstransports, utils
 ]
 import ../protocol/[enums, types]
-import std/[options, unittest, json, os, jsonutils, sequtils, strutils, sugar, strformat, osproc]
+import std/[options, unittest, json, os, jsonutils, sequtils, strutils, sugar, strformat]
 import json_rpc/[rpcclient]
 import chronicles
 import lspsocketclient
+import chronos/asyncproc
 
 
 suite "Nimlangserver":
@@ -50,10 +51,10 @@ suite "Nimlangserver":
     client.notify("textDocument/didOpen",
                   %createDidOpenParams("projects/hw/useRoot.nim"))
 
-    let prevSuggestPid = ls.projectFiles[hwAbsFile].waitFor.process.processID
+    let prevSuggestPid = ls.projectFiles[hwAbsFile].waitFor.process.pid
     let suggestParams = SuggestParams(action: saRestart, projectFile: hwAbsFile)
     let suggestRes = client.call("extension/suggest", %suggestParams).waitFor
-    let suggestPid = ls.projectFiles[hwAbsFile].waitFor.process.processID
+    let suggestPid = ls.projectFiles[hwAbsFile].waitFor.process.pid
 
     check prevSuggestPid != suggestPid
 

utils.nim

@@ -1,4 +1,4 @@
-import std/[unicode, uri, strformat, os, strutils, options, json, jsonutils, sugar]
+import std/[unicode, uri, strformat, os, strutils, options, json, jsonutils, sugar, net]
 import chronos, chronicles
 import "$nim/compiler/pathutils"
 import json_rpc/private/jrpc_sys
@@ -277,4 +277,12 @@ proc withTimeout*[T](fut: Future[T], timeout: int = 500): Future[Option[T]] {.as
   #Returns None when the timeout is reached and cancels the fut. Otherwise returns the Fut
   let timeoutFut = sleepAsync(timeout).map(() => none(T))
   let optFut = fut.map((r: T) => some r)
-  await either(optFut, timeoutFut)
+  await either(optFut, timeoutFut)
+
+proc getNextFreePort*(): Port= 
+  let s = newSocket()
+  s.bindAddr(Port(0), "localhost")
+  let (_, port) = s.getLocalAddr
+  s.close()
+  port
+