Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: StartProcess fails to keep Files alive after converting to []uintptr array #34858

Closed
mdempsky opened this issue Oct 12, 2019 · 1 comment
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mdempsky
Copy link
Member

As noted during discussion of #34810, os.StartProcess fails to use runtime.KeepAlive to ensure attr.Files stays alive past the call to syscall.StartProcess. As a consquence, the os.Files might be garbage collected, and the FDs can be reclaimed before they're used.

Below is a proof of concept of this issue (it fails most runs, but not always):

$ go run x.go
panic: 58 fork/exec /bin/true: bad file descriptor

goroutine 1 [running]:
main.main()
	/tmp/x.go:25 +0x2f6
exit status 2

$ cat x.go
package main

import (
	"fmt"
	"os"
	"runtime"
)

func main() {
	go func() {
		for {
			runtime.GC()
		}
	}()

	for i := 0; i < 100; i++ {
		p, err := os.StartProcess("/bin/true", nil, &os.ProcAttr{
			Files: []*os.File{
				mustOpen("/dev/null"),
				mustOpen("/dev/null"),
				mustOpen("/dev/null"),
			},
		})
		if err != nil {
			panic(fmt.Sprint(i, err))
		}
		p.Release()
	}
}

func mustOpen(name string) *os.File {
	f, err := os.Open(name)
	if err != nil {
		panic(err)
	}
	return f
}

/cc @ianlancetaylor

@mdempsky mdempsky added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 12, 2019
@mdempsky mdempsky added this to the Go1.14 milestone Oct 12, 2019
@gopherbot
Copy link

Change https://golang.org/cl/201198 mentions this issue: os: keep attr.Files alive when calling StartProcess

@golang golang locked and limited conversation to collaborators Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

2 participants