os/exec: Should `*exec.Cmd` export the waitDone channel ?

[sinloss]

What version of Go are you using (go version)?

go version go1.13.4 windows/amd64

Does this issue reproduce with the latest release?

Yes, it does.

What operating system and processor architecture are you using (go env)?

go env Output

set GO111MODULE=
set GOARCH=amd64
set GOBIN=C:\Lang\Go\bin
set GOCACHE=C:\Users\sinlo\AppData\Local\go-build
set GOENV=C:\Users\sinlo\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Lang\Gopath
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Lang\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Lang\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\sinlo\AppData\Local\Temp\go-build452985766=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
	"fmt"
	"io"
	"net"
	"os"
	"os/exec"
	"runtime"
)

func main() {
	sig := make(chan struct{})
	go func() {
		l, _ := net.Listen("tcp", "127.0.0.1:9527")
		sig <- struct{}{} // notify the main routine it's time to connect

		c, _ := l.Accept()

		var cmd *exec.Cmd
		if runtime.GOOS == "windows" {
			cmd = exec.Command("chcp.com")
		} else {
			cmd = exec.Command("ls")
		}
		cmd.Dir, _ = os.Getwd()
		cmd.Stdin = c
		cmd.Stdout = c
		cmd.Stderr = c

		err := cmd.Run()
		if err != nil {
			fmt.Println(err)
		}

                // notify the main routine the cmd has got executed, it's time to quit
		sig <- struct{}{}
	}()

	<-sig // wait for the signal of "time to connect"
	conn, _ := net.Dial("tcp", "127.0.0.1:9527")

	go io.Copy(conn, os.Stdin)
	go io.Copy(os.Stdout, conn)

	<-sig // wait for the signal of "time to quit"
}

It's a simple test case shown above. Sorry that I could not provide a link on play.golang.org, because I could not decide a proper executable that implemented by the NaCl. I tried yet failed on every single guess...

What did you expect to see?

Well, I expected it exit without my hitting the Enter.

What did you see instead?

Yet actually it just can't give in that simple, it holds on until I hit the Enter.

And

So I went through all the way down and finally found out that the unexported function os/exec.(*Cmd).stdin which was called by the os/exec.(*Cmd).Start found out the Stdin which was assigned by me is not an (*os.File) type, and then appended a function ( we'll just call it mister A ) who did all the io.Copy labor to the goroutine field.
Then, this hard-working function A was started as a goroutine by the os/exec.(*Cmd).Start along with another mate ( this is mister B ) appended by the os/exec.(*Cmd).writerDescriptor for a similar reason.

With all that happened, when the os/exec.(*Cmd).Wait() is called, it waits for the process to get done, and it then waited for the channel field errch to produce all the err returned by A and B. And just at here, the A stuck as the A is doing a hard work copying bytes from the Stdin which is assigned by me to the pw which is a newly created pipe by the os/exec.(*Cmd).stdin, and at this moment the Stdin field hasn't yet got any bytes from the other end of the network connection while at the meantime, the process was over, the pw was actually already closed.

So when I hit the Enter, the A finally found out that the pw to which it was trying to write data has already closed. Then the os/exec.(*Cmd).Wait() went on.

My question is

I think the whole process does make sense that the os/exec.(*Cmd).Wait() should wait for the read/write to complete. And yet I want to avoid hitting the Enter. So I'm wondering what if we provide a function to get a read-only channel of the channel field waitDone or make another channel to implement this? That channel could inform the user when the process is actually executed regardless of the status of A and B.

I'm available for this

If that is ok, I would be glad to submit a PR.

[toothrot]

I think you're right, the behavior here is described in the documentation for cmd.Stdin:

	// Stdin specifies the process's standard input.
	//
	// If Stdin is nil, the process reads from the null device (os.DevNull).
	//
	// If Stdin is an *os.File, the process's standard input is connected
	// directly to that file.
	//
	// Otherwise, during the execution of the command a separate
	// goroutine reads from Stdin and delivers that data to the command
	// over a pipe. In this case, Wait does not complete until the goroutine
	// stops copying, either because it has reached the end of Stdin
	// (EOF or a read error) or because writing to the pipe returned an error.
	Stdin io.Reader

I am not sure if exposing waitDone is the correct API for this. Cmd already has Process exported, so presumably Process.Wait() should give you similar information or control. In your example, does cmd.Process.Wait() do what you want?

Please let me know if I've misunderstood something here.

[ianlancetaylor]

This is basically a variant of #23019 and should perhaps be closed as a dup. Discussion should likely be on #23019 rather than on this new issue.

[toothrot]

Closed as dup of #23019