cmd icon indicating copy to clipboard operation
cmd copied to clipboard

StartWithStdin channel keeps blocking after process stops

Open joepadmiraal opened this issue 5 years ago • 11 comments

Thanks for maintaining go-cmd. I've got a C program that get's started with stdin. This allows the Go code to send instructions over stdin to the C program. However if the C program finishes, the cmd.Status chan does not seem to unblock. Is there something I'm doing wrong here?

Go code:

//Dummy reader which does nothing
type TestReader struct {
}

//Read get's called by go-cmd
func (rt *TestReader) Read(p []byte) (n int, err error) {
	return 0, nil
}

func NewTestReader() *TestReader {
	rt := TestReader{}
	return &rt
}

func buffered() {
	testCmd := cmd.NewCmd("seg")
	rt := NewTestReader()
	statusChan := testCmd.StartWithStdin(rt)

	// statusChan := testCmd.Start()

	ticker := time.NewTicker(1 * time.Second)

	go func() {
		for range ticker.C {
			status := testCmd.Status()
			fmt.Println(status.Stdout)
			fmt.Println(status.Stderr)
		}
	}()

	// Block waiting for command to exit, be stopped, or be killed. 
	// This does not unblock
	state := <-statusChan

	fmt.Println("statusChan final state")
	fmt.Println(state.Stdout)
	fmt.Println(state.Stderr)
}

C code:

#include <unistd.h>

int main(int argc, char* argv[])
{
    setvbuf(stdout, NULL, _IOLBF, 0);
    setvbuf(stderr, NULL, _IOLBF, 0);

    printf("C test\n");
    sleep(1);
    printf("1\n");
    sleep(1);
    printf("done\n");
}```

joepadmiraal avatar Nov 10 '20 12:11 joepadmiraal

Hi. Sorry, I don't understand the direction of programs. You said,

This allows the Go code to send instructions over stdin to the C program.

But in the sample code above, the C program is sending to the Go code?

daniel-nichter avatar Nov 15 '20 16:11 daniel-nichter

Hi, thanks for responding.

Yes, the C program above is a minimal example to demonstrate the issue. My actual program does read from stdin and write to stdout.

joepadmiraal avatar Nov 15 '20 18:11 joepadmiraal

Does Go code also read on stdin? If so, then it's like echo hi | go | c (where c is being run by go)?

By the way, when I ran the example code above, nothing blocked, but I'm not sure if it's a functional example because // statusChan := testCmd.Start() suggests the Go isn't running the C.

daniel-nichter avatar Nov 15 '20 22:11 daniel-nichter

The example does not read on stdin. The commented out code was left over from a test I did. The blocking happens with the row above statusChan := testCmd.StartWithStdin(rt)

I have created a small repository with a working example you can easily run: https://github.com/joepadmiraal/cmd-test

joepadmiraal avatar Nov 16 '20 07:11 joepadmiraal

@joepadmiraal, this is expected behaviour.

go-cmd uses exec.cmd.Wait internally, and https://golang.org/pkg/os/exec/#Cmd states that cmd.Wait, will not terminate until EOF or an error is read if you have a custom Stdin. Your TestReader always returns 0,nil, so this is expected behaviour.

Try this in your demo code, and it will no longer hang:

//Dummy reader which does nothing
type TestReader struct {
  start time.Time
}

//Read get's called by go-cmd
func (rt *TestReader) Read(p []byte) (n int, err error) {
   if time.Since(rt.start) > time.Second * 5 {
     err = io.EOF
   }
   return
}

hfrappier avatar Mar 29 '21 21:03 hfrappier

Ok, but doesn't that make it impossible to detect whether a program has stopped?

If I want to stop the program from the Go code I can send the EOF via the reader. But if the C program stops itself, it does not unblock.

joepadmiraal avatar Jan 13 '22 09:01 joepadmiraal

Ok, but doesn't that make it impossible to detect whether a program has stopped?

If I want to stop the program from the Go code I can send the EOF via the reader.

But if the C program stops itself, it does not unblock.

I agree, I have to use a hacky workaround to detect if the program is still running (using gopsutil). Would be nice to have a better solution there.

MarcStdt avatar Jan 13 '22 14:01 MarcStdt

This is an edge case with io.Pipe usage within exec.Cmd iteslf, not go-cmd:

package main

import (
        "fmt"
        "time"
        "os/exec"
)

//Dummy reader which does nothing
type TestReader struct {
  after time.Time
}

var sent int
//Read get's called by go-cmd
func (rt *TestReader) Read(p []byte) (n int, err error) {
  if !rt.after.IsZero() && time.Now().After(rt.after) {
    n=1
  }
  return
}

type TestWriter struct {}
func (rt* TestWriter) Write(p []byte) (n int, err error) {
  fmt.Println(string(p))
  n = len(p)
  return
}

func main() {
        oscmd := exec.Command("c/seg", "")
        fail := &TestReader{}
        pass := &TestReader{time.Now().Add(time.Second *5)}
        _,_ = fail,pass
        tw := &TestWriter{}
        oscmd.Stdin = fail
        //oscmd.Stdin = pass
        oscmd.Stdout = tw
        oscmd.Start()
        oscmd.Wait()
        fmt.Println("Finished")
}

I'd expect a zero-length payload to detect whether a pipe was broken as well (e.g TCP)

hfrappier avatar Jan 13 '22 23:01 hfrappier

I had a look at what exec.Cmd has to offer and it seems they have something called StdinPipe which can be used to solve the problem. I've updated my demo repository with an example of how this could work.

Would it make sense to change the go-cmd implementation to start using StdinPipe? Or maybe we can simply expose StdinPipe, StdoutPipe and StderrPipe?

joepadmiraal avatar Jan 21 '22 08:01 joepadmiraal

我查看了 exec.Cmd 必须提供的内容,似乎他们有一个StdinPipe可以用来解决问题的东西。我已经更新了我的演示存储库,并提供了一个说明其工作原理的示例。

更改 go-cmd 实现以开始使用是否有意义StdinPipe? 或者也许我们可以简单地暴露StdinPipe, StdoutPipeand StderrPipe

I also encountered the same problem , are you have resolved the issue? Looking forward to your sharing.

YongBig avatar Sep 25 '23 13:09 YongBig

How about using io.Pipe with go-cmd?

	reader, writer := io.Pipe()
	go func() {
		_, _ = io.Copy(writer, bytes.NewBufferString("hello from the other side\n"))
		// close immediately
		_ = writer.Close()
	}()
	statusChan := testCmd.StartWithStdin(reader)
	...

Will not block, the buffer is sent to the child process stdin

cr1cr1 avatar Oct 12 '23 08:10 cr1cr1