cmd/go: run does not close its own os.Stdout when the underlying process does so #39172

ribasushi · 2020-05-20T11:12:11Z

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

go version go1.14.2 darwin/amd64

Does this issue reproduce with the latest release?

✔️ ( fails the same way on multiple go versions )

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

macos Mojave

What did you do?

I have applications needing to communicate over multiple FIFO's in turn (due to boring/non-negotiable external requirements). The dance looks like:

$ ProducerProgram ... --outv0-xargs | xargs -0 ConsumerProgram

Where:

ProducerProgram creates a number of named pipes, prints their names, closes STDOUT, and starts writing to each FIFO in turn
xargs sees the EOF, starts the ConsumerProgram with the FIFOs as args and it proceeds reading from each FIFO in the same order

The trouble is that go build; ./ProducerProgram ... | works, while go run ./. ... | does not, as go run does not propagate the closure of os.Stdout until program exit... which can never happen because the FIFO reading never starts.

Below find a complete program that exhibits the problem as:

# this works
$ go build -o pipehang ./ ; ./pipehang | xargs -0 cat | wc -c
 33554432

# this hangs
$ go run ./ | xargs -0 cat | wc -c

Standalone program to reproduce

package main

import (
	"fmt"
	"log"
	"os"

	"golang.org/x/sys/unix"
)

func main() {
	ec := make(chan error)
	go errorHandler(ec)

	defer os.Remove("fifo1")
	defer os.Remove("fifo2")

	// do it again :(
	// AN ASIDE - it would be great if I could mkfifo with "force" instead of having to cleanup ahead of time
	os.Remove("fifo1")
	os.Remove("fifo2")

	ec <- unix.Mkfifo("fifo1", 0600)
	ec <- unix.Mkfifo("fifo2", 0600)

	// AN ASIDE/maybe another bug: if I open with O.WRONLY on macos - everything hangs on this line
	fh1, err := os.OpenFile("fifo1", os.O_RDWR, 0)
	ec <- err

	fh2, err := os.OpenFile("fifo2", os.O_RDWR, 0)
	ec <- err

	_, err = fmt.Fprintf(
		os.Stdout,
		"%s\x00%s\x00",
		"fifo1",
		"fifo2",
	)
	ec <- err

	// THIS IS THE BUG - we signal to xargs "we are done", but
	// go run will not close its own Stdout in turn
	ec <- os.Stdout.Close()

	sixteenMegs := make([]byte, 16*1024*1024)
	_, err = fh1.Write(sixteenMegs)
	ec <- err
	ec <- fh1.Close()

	_, err = fh2.Write(sixteenMegs)
	ec <- err
	ec <- fh2.Close()

	return
}

func errorHandler(c <-chan error) {
	for {
		err, chanOpen := <-c
		if !chanOpen {
			return
		}
		if err != nil {
			log.Fatal(err)
		}
	}
}

The text was updated successfully, but these errors were encountered:

as · 2020-05-20T16:48:31Z

One problem: os.Stdout is assignable, so there's no guarantee that it would close file descriptor 1.

ALTree · 2020-05-20T16:59:01Z

Note that go run is just a convenience wrapper that builds to a temporary file and launches the binary. It's not supposed to work exactly like running the actual binary, and I also don't think you should use it in production or for serious stuff. Especially when you care about things like the process return code or pipes working correctly.

ianlancetaylor · 2020-05-20T21:48:28Z

Yeah, if go run doesn't work for you, I would definitely recommend using go build x && ./x instead.

ribasushi · 2020-05-20T22:00:53Z

Acknowledging go run is not something to use for production purposes, isn't this just a bug in whatever implements the fork/exec or whatever it is that go uses?

I could dig further into this to find what exactly needs fixing, I just ran out of time figuring the initial cause. What I would like to hear first is whether this is something that the core team considers an actual error in behavior.

P.S. In my specific case I will have to devise some sort of workaround in any case, because telling users "check out this repo and play with it, but oh wait, you can't use go run, for reasons" is too far on the "lose their interest" scale.

ianlancetaylor · 2020-05-20T23:10:46Z

It's not a bug in the implementation of fork/exec, which in this case is the os/exec package (https://golang.org/pkg/os/exec). That lets you set standard output to whatever you want. go run just sets the standard output of the child process to its own standard output, and does nothing else.

As you say, that doesn't work for programs that expect to be able to close their own standard output and have that close be seen by processes later in the pipeline.

ALTree · 2020-05-21T06:33:34Z

P.S. In my specific case I will have to devise some sort of workaround in any case, because telling users "check out this repo and play with it, but oh wait, you can't use go run, for reasons" is too far on the "lose their interest" scale.

It seems to me that you're over-blowing this... You wanted to write:

cd into the project folder, then run it:

$ go run ./. ... | xargs ...

Instead, you have to write:

cd into the project folder, build the program and then run it:

$ go build
$ pipehang | xargs ...

I even like the 2nd version better! I find the go run ./... pattern quite ugly, IMO once you have a multiple-files project go build should be preferred. I only use go run when I can write go run main.go; otherwise I always build.

ribasushi · 2020-05-21T08:12:15Z

@ianlancetaylor looking through the code more I see what is happening... It is a really unfortunate design decision that go run was not implemented through a real execve(), but I appreciate that changing this at the current stage is no longer tenable. I'll close the ticket.

@ALTree the "you are holding it wrong" tone is really unnecessary. The project in question requires/invites the user to tweak the source. The extra go build step, is just that - an extra step. The answer is a "time-to-first-write" timeout with a helpful error message "did you run it like X? That unfortunately doesn't work, use the verbose Y instead, sorry!"

cagedmantis changed the title ~~go run does not close its own os.Stdout when the underlying process does so~~ cmd/go: run does not close its own os.Stdout when the underlying process does so May 20, 2020

ribasushi closed this as completed May 21, 2020

golang locked and limited conversation to collaborators May 21, 2021

gopherbot added the FrozenDueToAge label May 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/go: run does not close its own os.Stdout when the underlying process does so #39172

cmd/go: run does not close its own os.Stdout when the underlying process does so #39172

ribasushi commented May 20, 2020

as commented May 20, 2020

ALTree commented May 20, 2020 •

edited

ianlancetaylor commented May 20, 2020

ribasushi commented May 20, 2020

ianlancetaylor commented May 20, 2020

ALTree commented May 21, 2020 •

edited

ribasushi commented May 21, 2020 •

edited

cmd/go: run does not close its own os.Stdout when the underlying process does so #39172

cmd/go: run does not close its own os.Stdout when the underlying process does so #39172

Comments

ribasushi commented May 20, 2020

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

Does this issue reproduce with the latest release?

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

What did you do?

as commented May 20, 2020

ALTree commented May 20, 2020 • edited

ianlancetaylor commented May 20, 2020

ribasushi commented May 20, 2020

ianlancetaylor commented May 20, 2020

ALTree commented May 21, 2020 • edited

ribasushi commented May 21, 2020 • edited

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

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

ALTree commented May 20, 2020 •

edited

ALTree commented May 21, 2020 •

edited

ribasushi commented May 21, 2020 •

edited