go: os/exec: exec.command has memory leak

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?

I am using exec.Command() in my code and observed that there is a memory leak in this call. Actually my requirement is my program will continuously runs on machine as a daemon, so memory used by program goes high after 2 to 3 days from initial 10MB to 200MB and increasing continuously unless i restart the daemon.

Below is the small test program which i can show you the memory leak in exec.Command. Here initially program taking 960-1000 KB, once it is executed some commands memory goes high to 1200KB+…

package main

import (
	"fmt"
	"os"
	"os/exec"
	"runtime"
	"strings"
)

func main() {
	//to get current executable PID
	strPid := ExecuteCommand("pidof", os.Args[0])
	//to get Memory used by executable
	strMem := ExecuteCommand("ps", "-p", strings.TrimSpace(strPid), "-o", "rss")
	fmt.Println(" Initial Memory in KB ", string(strMem[:]))
	for i := 0; i < 10; i++ {
		cmd := exec.Command("sleep", "1")
		err := cmd.Start()
		if err != nil {
			fmt.Println(err)
		}
		err = cmd.Wait()
		fmt.Printf("Command finished with error: %v\n", err)
	}
	strMem = ExecuteCommand("ps", "-p", strings.TrimSpace(strPid), "-o", "rss")
	fmt.Println("\nAfter For loop, Memory in KB ", string(strMem[:]))
	//running garbage collector
	runtime.GC()
	strMem = ExecuteCommand("ps", "-p", strings.TrimSpace(strPid), "-o", "rss")
	fmt.Println("\nFinal Memory in KB ", string(strMem[:]))

}

func ExecuteCommand(str string, params ...string) string {
	output, err := exec.Command(str, params...).Output()
	if err != nil {
		fmt.Println(err)
		return ""
	}
	return string(output[:])

}

About this issue

Original URL
State: closed
Created 7 years ago
Comments: 20 (11 by maintainers)

Most upvoted comments

You are leaking a goroutine. The goroutine created in line 64 will never exit as timer.C will never be closed and thus the goroutine holds onto some variables forever.

If you replace

timer := time.NewTimer(time.Millisecond * time.Duration(cmdTimeout))
go func(timer *time.Timer, cmd *exec.Cmd) {
	for _ = range timer.C {
		stdinWrite.Close()
		stdoutRead.Close()
		stderrRead.Close()
		errKill := cmd.Process.Signal(os.Kill)
		if errKill != nil {
			AgentLog.Error("Failed to stop the timer, error info : ", errKill)
		}
	}
}(timer, cmd)

with this

timer := time.AfterFunc(time.Millisecond * time.Duration(cmdTimeout), func() {
	stdinWrite.Close()
	stdoutRead.Close()
	stderrRead.Close()
	errKill := cmd.Process.Signal(os.Kill)
	if errKill != nil {
		AgentLog.Error("Failed to stop the timer, error info : ", errKill)
	}
})

both the goroutine and the variables can be garbage collected.

The reason for this (as mentioned at the beginning) is that timer.C will never be closed, since a time.Timer can be reused, and a range loop over a channel runs until the channel is closed (or break is used). By using time.AfterFunc the code is not only much clearer, but also the callback won’t even be called and no goroutine created when Stop() is called before the time elapsed

nussjustin on Jul 13, 2017