Skip to content

os/exec: exec.command has memory leak #20980

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

Closed
Durgababu opened this issue Jul 11, 2017 · 10 comments
Closed

os/exec: exec.command has memory leak #20980

Durgababu opened this issue Jul 11, 2017 · 10 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@Durgababu
Copy link

Durgababu commented Jul 11, 2017

Please answer these questions before submitting your issue. Thanks!

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

1.8.3

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

linux

What did you do?

we are using exec.Command() method many places. And we observed that there is memory leak in each call to this. Even after running running.GC(), it is not getting freeing up.

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

We did memory profile using pprof package. It is showing leak in below code:
os/exec.Command(/usr/local/go/src/os/exec/exec.go:133)
os/exec.(*Cmd).start(usr/local/go/src/os/exec/exec.go:368)
os/exec.(*Cmd).start(usr/local/go/src/os/exec/exec.go:359)

@ALTree
Copy link
Member

ALTree commented Jul 11, 2017

Are you using *Cmd.Start? Are you calling Wait?

The Wait method will return the exit code and release associated resources once the command exits.

http://tip.golang.org/pkg/os/exec/#Cmd.Start

cmd := exec.Command("sleep", "5")
err := cmd.Start()
if err != nil {
    log.Fatal(err)
}
log.Printf("Waiting for command to finish...")
err = cmd.Wait()
log.Printf("Command finished with error: %v", err)

Note the err = cmd.Wait() line.

@ALTree ALTree added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 11, 2017
@ALTree ALTree changed the title os/exec : exec.command has memory leak os/exec: exec.command has memory leak Jul 11, 2017
@Durgababu
Copy link
Author

We are using Start() as well as Wait(), still we have memory leak.

@mvdan
Copy link
Member

mvdan commented Jul 11, 2017

@Durgababu please provide a short program that reproduces the problem. Otherwise it's very hard to tell whether there is an issue with how you're using the exec package.

@Durgababu
Copy link
Author

Durgababu commented Jul 11, 2017

str := "df -lPT -B1 -x iso9660"
cmdTimeout := int64(30000)
binary, err := exec.LookPath("sh")
if err != nil {
return err.Error()
}

cmd := exec.Command(binary)
stdinWrite, _ := cmd.StdinPipe()
writer := bufio.NewWriter(stdinWrite)
stdoutRead, _ := cmd.StdoutPipe()
reader_out := bufio.NewReader(stdoutRead)
stderrRead, _ := cmd.StderrPipe()
reader_err := bufio.NewReader(stderrRead)

errStart := cmd.Start()
if errStart != nil {
Log.Error("errStart:", errStart)
}
timer := time.NewTimer(time.Millisecond * time.Duration(cmdTimeout))
go func(timer *time.Timer, cmd *exec.Cmd) {
for _ = range timer.C {
errKill := cmd.Process.Signal(os.Kill)
if errKill != nil {
Log.Error("Failed to stop the timer, error info : ", errKill)
}
}
}(timer, cmd)
writer.WriteString(str)
writer.Flush()
stdinWrite.Close()
randomBytes := &bytes.Buffer{}
errorBytes := &bytes.Buffer{}
var linearr string
for {
var buffer []byte
buffer = make([]byte, 1024)
len1, errRead := reader_out.Read(buffer)
if errRead == nil {
if len1 != 0 {
randomBytes.WriteString(string(buffer[0:len1]))
}
} else {
break
}
}
linearr = strings.TrimSpace(string(randomBytes.Bytes()))
var strerr string
for {
var buffer []byte
buffer = make([]byte, 1024)
len2, errRead := reader_err.Read(buffer)
if errRead == nil {
if len2 != 0 {
errorBytes.WriteString(string(buffer[0:len2]))
}
} else {
break
}
}
strerr = strings.TrimSpace(string(errorBytes.Bytes()))
cmd.Wait()
timer.Stop()
stderrRead.Close()
stdoutRead.Close()

@ALTree
Copy link
Member

ALTree commented Jul 11, 2017

cmd.Process.Signal(os.Kill)

Are you sending SIGKILLs to the processes spawned for your Cmds?

cmd.Wait()

Also you're not checking the error returned by cmd.Wait. Please do so. Are the Wait calls exiting without errors or not?

@Durgababu
Copy link
Author

Durgababu commented Jul 11, 2017

cmd.Process.Signal(os.Kill)

yes. We are sending SIGKILL to stop the command execution if it reaches timeout.

@Durgababu
Copy link
Author

Also you're not checking the error returned by cmd.Wait. Please do so. Are the Wait calls exiting without errors or not?

We added error checking for cmd.Wait(), it is exiting without errors(nil).

@Durgababu
Copy link
Author

We did memory profile for above code using pprof package. It is showing leak in below lines of code:
os/exec.Command(/usr/local/go/src/os/exec/exec.go:133)
os/exec.(*Cmd).start(usr/local/go/src/os/exec/exec.go:368)
os/exec.(*Cmd).start(usr/local/go/src/os/exec/exec.go:359)

can someone help?

@ALTree
Copy link
Member

ALTree commented Jul 11, 2017

IMHO you should ask for help in an appropriate place (see the Questions wiki page; it has a list of good places for asking for help on your code).

If you ask for help in one of those places, please try to post an auto-contained example. The code you posted above is not auto-contained, because I can't just copy-past it in a text-file and run it. It has no main, the code includes a return but it's not in a function, and so on.

It's not even clear to me that the leak is real because you're not explaining how you're measuring memory usage. How do you know there's a leak? What pprof command are you using? Please include all this information if you ask for help, as I suggested, on one of the places I linked above.

@mvdan
Copy link
Member

mvdan commented Jul 11, 2017

I agree with @ALTree. The discussion could go on, but this question doesn't belong in the issue tracker. I'm going to close this for now.

Feel free to open a new issue if you believe you found a bug. But please, as Alberto says, do provide a reproducible example - otherwise it's likely just a question.

@mvdan mvdan closed this as completed Jul 11, 2017
@golang golang locked and limited conversation to collaborators Jul 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants