diff options
-rw-r--r-- | changelog.md | 3 | ||||
-rw-r--r-- | lib/pure/osproc.nim | 6 | ||||
-rw-r--r-- | lib/windows/winlean.nim | 2 | ||||
-rw-r--r-- | tests/stdlib/tosproc.nim | 26 |
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) |