summary refs log tree commit diff stats
diff options
context:
space:
mode:
-rw-r--r--changelog.md3
-rw-r--r--lib/pure/osproc.nim6
-rw-r--r--lib/windows/winlean.nim2
-rw-r--r--tests/stdlib/tosproc.nim26
4 files changed, 36 insertions, 1 deletions
diff --git a/changelog.md b/changelog.md
index 7907432a7..f1ffb4a3a 100644
--- a/changelog.md
+++ b/changelog.md
@@ -86,6 +86,9 @@
   an uninitialized `DateTime`, the exceptions are `==` and `$` (which returns `"Uninitialized DateTime"`). The proc `times.isInitialized`
   has been added which can be used to check if a `DateTime` has been initialized.
 
+- Fix a bug where calling `close` on io streams in osproc.startProcess was a noop and led to
+  hangs if a process had both reads from stdin and writes (eg to stdout).
+
 ## Language changes
 - In newruntime it is now allowed to assign discriminator field without restrictions as long as case object doesn't have custom destructor. Discriminator value doesn't have to be a constant either. If you have custom destructor for case object and you do want to freely assign discriminator fields, it is recommended to refactor object into 2 objects like this:
   ```nim
diff --git a/lib/pure/osproc.nim b/lib/pure/osproc.nim
index 5839349fe..f90b310ea 100644
--- a/lib/pure/osproc.nim
+++ b/lib/pure/osproc.nim
@@ -441,7 +441,11 @@ when defined(Windows) and not defined(useNimRtl):
       handle: Handle
       atTheEnd: bool
 
-  proc hsClose(s: Stream) = discard # nothing to do here
+  proc hsClose(s: Stream) =
+    # xxx here + elsewhere: check instead of discard; ignoring errors leads to
+    # hard to track bugs
+    discard FileHandleStream(s).handle.closeHandle
+
   proc hsAtEnd(s: Stream): bool = return FileHandleStream(s).atTheEnd
 
   proc hsReadData(s: Stream, buffer: pointer, bufLen: int): int =
diff --git a/lib/windows/winlean.nim b/lib/windows/winlean.nim
index 9beccc0eb..bd4cfdca7 100644
--- a/lib/windows/winlean.nim
+++ b/lib/windows/winlean.nim
@@ -31,6 +31,8 @@ type
   ULONG* = int32
   PULONG* = ptr int
   WINBOOL* = int32
+    ## `WINBOOL` uses opposite convention as posix, !=0 meaning success.
+    # xxx this should be distinct int32, distinct would make code less error prone
   DWORD* = int32
   PDWORD* = ptr DWORD
   LPINT* = ptr int32
diff --git a/tests/stdlib/tosproc.nim b/tests/stdlib/tosproc.nim
index fc65606f6..73cec0cb7 100644
--- a/tests/stdlib/tosproc.nim
+++ b/tests/stdlib/tosproc.nim
@@ -93,3 +93,29 @@ else:
       removeFile(exePath)
     except OSError:
       discard
+
+  import std/streams
+  block: # test for startProcess (more tests needed)
+    # bugfix: windows stdin.close was a noop and led to blocking reads
+    proc startProcessTest(command: string, options: set[ProcessOption] = {
+                    poStdErrToStdOut, poUsePath}, input = ""): tuple[
+                    output: TaintedString,
+                    exitCode: int] {.tags:
+                    [ExecIOEffect, ReadIOEffect, RootEffect], gcsafe.} =
+      var p = startProcess(command, options = options + {poEvalCommand})
+      var outp = outputStream(p)
+      if input.len > 0: inputStream(p).write(input)
+      close inputStream(p)
+      result = (TaintedString"", -1)
+      var line = newStringOfCap(120).TaintedString
+      while true:
+        if outp.readLine(line):
+          result[0].string.add(line.string)
+          result[0].string.add("\n")
+        else:
+          result[1] = peekExitCode(p)
+          if result[1] != -1: break
+      close(p)
+
+    var result = startProcessTest("nim r --hints:off -", options = {}, input = "echo 3*4")
+    doAssert result == ("12\n", 0)