nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.57k stars 1.47k forks source link

osproc: execProcess and startProcess hang forever (stuck on streams.readLine) #9953

Open timotheecour opened 5 years ago

timotheecour commented 5 years ago

/cc @Araq @dom96 this is a serious usability blocker

I'm on OSX

any osproc routine that captures stdout gets stuck forever.

nim c -r -d:case1 $timn_D/tests/nim/all/t0040.nim

before
^CTraceback (most recent call last)
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim(29) t0040
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim(23) main
/Users/timothee/git_clone/nim/Nim/lib/pure/osproc.nim(365) execProcess
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(280) readLine
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(162) readChar
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(87) readData
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(413) fsReadData
SIGINT: Interrupted by Ctrl-C.

nim c -r -d:case2 $timn_D/tests/nim/all/t0040.nim

before
^CSIGINT: Interrupted by Ctrl-C.
Traceback (most recent call last)
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim(29) t0040
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim(27) main
/Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim(16) startProcessWrapper
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(306) readLine
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(75) atEnd
/Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim(408) fsAtEnd
/Users/timothee/git_clone/nim/Nim/lib/system/sysio.nim(233) endOfFile
SIGINT: Interrupted by Ctrl-C.
import osproc
import streams

let cmd = "/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl"

proc startProcessWrapper()=
  var p = startProcess(command = cmd, options={poStdErrToStdOut})
  defer: close(p)
  let sub = outputStream(p)
  while true:
    echo "before"
    let line = readLine(sub)
    echo line
    echo "after"

proc main()=
  when defined(case1):
    echo "before"
    let ret = execProcess(cmd, options={poStdErrToStdOut})
    echo "after"
    echo ret
  when defined(case2):
    startProcessWrapper()

main()

other languages don't have this problem

in D:

import std.process;
import std.stdio;

void main(){
  auto ret = execute("/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl");
  writeln("ret:", ret);
}
rdmd $timn_D/tests/nim/all/t0039.d
ret:Tuple!(int, "status", string, "output")(0, "")

in C++:

#include <cstdio>
#include <iostream>
#include <memory>
#include <stdexcept>
#include <string>
#include <array>

std::string exec(const char* cmd) {
    std::array<char, 128> buffer;
    std::string result;
    std::shared_ptr<FILE> pipe(popen(cmd, "r"), pclose);
    if (!pipe) throw std::runtime_error("popen() failed!");
    while (!feof(pipe.get())) {
        if (fgets(buffer.data(), 128, pipe.get()) != nullptr)
            result += buffer.data();
    }
    return result;
}

int main(){
  auto ret = exec("'/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl'");
  std::cout << "ret:" << ret << std::endl;
}

clang++ -o /tmp/z01 -std=c++14 -g -ferror-limit=7 $timn_D/tests/nim/all/t0041.cpp

/tmp/z01
ret:

note

timotheecour commented 5 years ago

note: since execProcess wraps startProcess, it's only worth looking at -d:case2

here's backtrace with lldb:

nim c --debugger:native -o:/tmp/nim//app -d:case2 /Users/timothee/git_clone//nim//timn//tests/nim/all/t0040.nim
 lldb /tmp/nim//app
(lldb) target create "/tmp/nim//app"
Current executable set to '/tmp/nim//app' (x86_64).
(lldb) r
Process 80120 launched: '/tmp/nim/app' (x86_64)
before
Process 80120 stopped
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
    frame #0: 0x00007fff6699ab9e libsystem_kernel.dylib __read_nocancel + 10
libsystem_kernel.dylib`__read_nocancel:
->  0x7fff6699ab9e <+10>: jae    0x7fff6699aba8            ; <+20>
    0x7fff6699aba0 <+12>: movq   %rax, %rdi
    0x7fff6699aba3 <+15>: jmp    0x7fff66999e67            ; cerror_nocancel
    0x7fff6699aba8 <+20>: retq
Target 0: (app) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = signal SIGSTOP
  * frame #0: 0x00007fff6699ab9e libsystem_kernel.dylib __read_nocancel + 10
    frame #1: 0x00007fff668ef87b libsystem_c.dylib _sread + 16
    frame #2: 0x00007fff668eeed1 libsystem_c.dylib __srefill1 + 24
    frame #3: 0x00007fff668eeff1 libsystem_c.dylib __srget + 14
    frame #4: 0x00007fff668e8595 libsystem_c.dylib fgetc + 52
    frame #5: 0x000000010001014c app endOfFile_OpxMRqWNQzmofyVQsNQqVA(f=0x00007fff9966e030) + 92 at /Users/timothee/git_clone/nim/Nim/lib/system/sysio.nim:233
    frame #6: 0x000000010001a3ae app fsAtEnd_Aq3rxetdTkDpVSXU7JqLPQ(s=0x000000010014edc8) + 126 at /Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim:408
    frame #7: 0x000000010001ae01 app atEnd_5ebsEcBZaoqxCkfDcpzdgg(s=0x000000010014edc8) + 97 at /Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim:75
    frame #8: 0x000000010001afe8 app readLine_KN3I0OGVr9bF9acs3y6DWpiA(s=0x000000010014edc8) + 120 at /Users/timothee/git_clone/nim/Nim/lib/pure/streams.nim:306
    frame #9: 0x00000001000127fa app startProcessWrapper_caZgCPLQenYzpHlyZdOOig_2 + 410 at /Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim:21
    frame #10: 0x0000000100012abd app main_caZgCPLQenYzpHlyZdOOig + 77 at /Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim:32
    frame #11: 0x0000000100012cd8 app NimMainModule + 184 at /Users/timothee/git_clone/nim/timn/tests/nim/all/t0040.nim:34
    frame #12: 0x0000000100012c19 app NimMainInner + 9 at /Users/timothee/git_clone/nim/Nim/lib/system.nim:3147
    frame #13: 0x0000000100012d1a app NimMain + 42 at /Users/timothee/git_clone/nim/Nim/lib/system.nim:3155
    frame #14: 0x0000000100012d67 app main(argc=1, args=0x00007ffeefbfc4f0, env=0x00007ffeefbfc500) + 71 at /Users/timothee/git_clone/nim/Nim/lib/system.nim:3162
    frame #15: 0x00007fff6686108d libdyld.dylib start + 1
(lldb)
alaviss commented 5 years ago

Do you have any easily reproducible sample that only requires POSIX tools? Not all of us have Sublime Text to test this with.

Araq commented 5 years ago

These APIs are heavily used. By testament, on OSX too. Nothing hangs. Maybe it's the space in your path?

timotheecour commented 5 years ago

i'm not making it up :) not a space in path issue, same thing after cp "/Applications/Sublime Text.app/Contents/SharedSupport/bin/subl" /tmp/z01 and trying on /tmp/z01

timotheecour commented 5 years ago

@alaviss > Do you have any easily reproducible sample that only requires POSIX tools? Not all of us have Sublime Text to test this with.

not yet but pretty sure it's not specific to sublime text ; it's got to do with stdout buffering and whether application returns no output at all if I were to guess

timotheecour commented 5 years ago

that could explain the bug ; it's still a bug IMO as both D and C++ work in that case.

alaviss commented 5 years ago

Here's my speculation: startProcess creates a pipe between the child and the parent, which causes the child's stdin to no longer be connected to the console, which causes sublime to enter the subl stdin buffer.

I think a partial version of poParentStreams that forwards only the stdin may work.

alaviss commented 5 years ago

Here's my PoC:

child.nim:

import terminal

if isatty stdin:
  echo "Hi!"
else:
  while true: discard

parent.nim:

import osproc

let ret = execProcess("./child", options={poStdErrToStdOut})

This will hang indefinitely unless poParentStreams is passed to execProcess

timotheecour commented 5 years ago

@alaviss thanks for the reply; but passing poParentStreams crashes:

/Users/timothee/git_clone/nim/Nim/lib/pure/osproc.nim(360) execProcess /Users/timothee/git_clone/nim/Nim/lib/pure/osproc.nim(1251) outputStream /Users/timothee/git_clone/nim/Nim/lib/system.nim(3930) failedAssertImpl /Users/timothee/git_clone/nim/Nim/lib/system.nim(3923) raiseAssert /Users/timothee/git_clone/nim/Nim/lib/system.nim(2969) sysFatal Error: unhandled exception: /Users/timothee/git_clone/nim/Nim/lib/pure/osproc.nim(1251, 17) poParentStreams notin p.options API usage error: stream access not allowed when you use poParentStreams [AssertionError] Error: execution of an external program failed: '/Users/timothee/git_clone/nim/timn/tests/nim/all/t0044 '

but I'm guessing you mean the code is osproc would need to implement this, correct?

I think a partial version of poParentStreams that forwards only the stdin may work.

alaviss commented 5 years ago

Yes, that's why I propose a poParentStdin instead to allow forwarding only stdin. poParentStreams will forward both stdin and stdout, so you can't capture the output.

timotheecour commented 5 years ago

In https://dlang.org/phobos/std_process.html we can pass directly the Files corresponding to stdin, stdout, stderr (which could be either stdout etc, or pointing to an actual file etc)

@trusted Pid spawnProcess(scope const(char[])[] args, File stdin = std.stdio.stdin, File stdout = std.stdio.stdout, File stderr = std.stdio.stderr, const string[string] env = null, Config config = Config.none, scope const char[] workDir = null); 

seems both simpler and more flexible than what we have in osproc:

proc startProcess(command: string; workingDir: string = ""; args: openArray[string] = []; env: StringTableRef = nil; options: set[ProcessOption] = {poStdErrToStdOut})

(i'm taling only about aspect of selecting stdin/stdout/stderr)

timotheecour commented 5 years ago

@alaviss thanks a bunch for your help in diagnosing this!, that seems to fix this bug, see https://github.com/nim-lang/Nim/pull/9963 for WIP

genotrance commented 5 years ago

Not sure if https://github.com/nim-lang/Nim/issues/8648 is related.

zah commented 5 years ago

I agree the process API needs to be reworked. I've filed a similar issue here: https://github.com/nim-lang/Nim/issues/7999

ghost commented 5 years ago

These APIs are heavily used. By testament, on OSX too. Nothing hangs. Maybe it's the space in your path?

I have this problem too: on nimforum.

davidgarland commented 5 years ago

Is there any workaround or alternative library for getting a setup like this to work currently? Or are we stuck until the API is reworked?

timotheecour commented 5 years ago

I've solved this issue and other problems related to osproc (eg https://github.com/nim-lang/Nim/issues/7999) by doing a full rewrite inspired from D's std/process.d approach (similar to what I had earlier described in https://github.com/nim-lang/Nim/issues/7999#issuecomment-447281710 but a bit more flexible). I don't have time/energy for a PR on this at the moment (notably, only works on linux/osx, didn't test on windows, and not PR ready) but can share a bit more details if anyone is interested. The API is flexible, allowing one to redirect stdin/stdout/stderr independently (as opposed to all or nothing poParentStreams) to a file, pipe or other descriptor, along with other improvements, and I'm quite happy with the result. So my advice for whoever wants to take on the task of improving osproc is to look closely at D's std.process

alaviss commented 5 years ago

So what is the API that you've arrived at? I'm interested :)

zah commented 5 years ago

@timotheecour, would you post your new code as a github repo? Perhaps the community can help to develop the Windows version and to integrate it with the async libraries. I'd like to have good process management support in our Chronos library for example.

alehander92 commented 5 years ago

i also wished independent parent streams, so this would be good :O

zevv commented 5 years ago

This one is not related to https://github.com/nim-lang/Nim/issues/8648. In this case the compiler generates too much output blocks because the pipe is full.

hoijui commented 1 year ago

I wrote a tool that makes heavy use of osproc, and this makes it unusable. I am considering switching the whole software to rust because of it. Please, fix this!

hoijui commented 1 year ago

I am writing a CLI application that uses stdin, stdout and stderr heavily. it also executes a lot of child processes, which also use them. without poParentStreams, the child prcesses hang/freeze my whole software. with poParentStreams, the child processes stdout and stderr output pollute my main softwares stdout and stderr. both are breaking situations.

hoijui commented 1 year ago

finally found a workaround that works for me!

It is essential to close stdin and stderr before trying to read stdout, to prevent hang/freeze:

let process = osproc.startProcess(
  command = "child-cmd",
  args = [],
  env = nil,
  options = {poUsePath})
process.inputStream.close() # NOTE **Essential** - This prevents hanging/freezing when reading stdout below
process.errorStream.close() # NOTE **Essential** - This prevents hanging/freezing when reading stdout below
let (lines, exCode) = process.readLines()
process.close() # This is probably not required