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

syscall: StartProcess exec status can be lost when fds are permuted #14979

Closed
millerresearch opened this issue Mar 26, 2016 · 1 comment
Closed

syscall: StartProcess exec status can be lost when fds are permuted #14979

millerresearch opened this issue Mar 26, 2016 · 1 comment
Milestone

Comments

@millerresearch
Copy link

@millerresearch millerresearch commented Mar 26, 2016

  1. What version of Go are you using (go version)?
    master
  2. What operating system and processor architecture are you using (go env)?
    tried on plan9_arm, linux_amd64, and darwin_amd64
  3. What did you do?
package main

import (
    "os"
)

func main() {
    attr := &os.ProcAttr{Files: []*os.File{os.Stdin, os.Stdout, os.Stderr}}
    _, e := os.StartProcess("/no/such/command", nil, attr)
    if e != nil {
        println("1 FAIL: " + e.Error())
    } else {
        println("1 OK")
    }
    attr = &os.ProcAttr{Files: []*os.File{os.Stdin, os.Stderr, os.Stdout}}
    _, e = os.StartProcess("/no/such/command", nil, attr)
    if e != nil {
        println("2 FAIL: " + e.Error())
    } else {
        println("2 OK")
    }
}
  1. What did you expect to see?
1 FAIL: fork/exec /no/such/command: ...
2 FAIL: fork/exec /no/such/command: ...
  1. What did you see instead?
1 FAIL: fork/exec /no/such/command: ...
2 OK

In syscall.forkAndExecInChild, a comment don't stomp on pipe refers to avoiding a collision between nextfd (a temporary file descriptor used for permuting the order of fds for the child) and pipe (the file descriptor of the pipe used to report exec status back to the parent). Unfortunately the code doesn't quite live up to the promise of the comment.

Moving the referenced code to a more strategic place in the loop body is probably the tidiest correction. A suggested CL will follow shortly.

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 26, 2016

CL https://golang.org/cl/21184 mentions this issue.

@ianlancetaylor ianlancetaylor added this to the Go1.7 milestone Mar 27, 2016
@gopherbot gopherbot closed this in 1f0bebc Mar 29, 2016
@golang golang locked and limited conversation to collaborators Mar 29, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.