Reading from a StdoutPipe() in Go freezes

user_2457519 0
Viewed 1069

I am trying to read from the Stdout of a command, but once every (approx.) 50 times it freezes.

func runProcess(process *exec.Cmd) (string, string, error) {
    var stdout strings.Builder
    var stderr string
    process := exec.Command(programPath, params...)

    go func() {
        pipe, err := process.StderrPipe()
        if err != nil {
            return
        }

        buf, err := io.ReadAll(pipe)
        if err != nil {
            log.Warn("Error reading stderr: %v", err)
        }
        stderr = string(buf)
    }()

    pipe, err := process.StdoutPipe()
    if err = process.Start(); err != nil {
        return "", "", err
    }

    buf := make([]byte, 1024)
    read, err := pipe.Read(buf)            // it reads correctly from the pipe
    for err == nil && read > 0 {
        _, err = stdout.Write(buf[:read])
        read, err = pipe.Read(buf)         // this is where is stalls
    }
    if err = process.Wait(); err != nil {
        return stdout.String(), stderr, err
    }

    return stdout.String(), stderr, nil
}

I've tried to use stdout, err := io.ReadAll(pipe) to read everything at once instead of reading chunks, but I get the same behaviour. The program that is called seems to be executed successfully. Its logfile is created and it is complete. Plus, first time when I read from the pipe (before the loop), all the output is there. But inside the loop, when the .Read() is called for the second time and it should return an EOF (the output is smaller than 1024 bytes), it freezes.

go stdout
1 Answers

There are many race conditions in this code. In general, if you create a goroutine, there should be some kind of synchronization--like a chan, sync.Mutex, sync.WaitGroup, or atomic.

Fix the race conditions.

  1. Call StderrPipe() before calling Start(). The code does not do this.

  2. Wait for the goroutine to finish before returning.

The race condition could corrupt the exec.Cmd structure... which could mean that it leaks a pipe, which would explain why Read() hangs (because a write end of the pipe wasn't closed).

As a rule of thumb, always fix race conditions. Consider them to be high-priority bugs.

Here is a sketch of how you could write it without race conditions:

func runProcess(process *exec.Cmd) (stdout, stderr string, err error) {
    outPipe, err := process.StdoutPipe()
    if err != nil {
        return "", "", err
    }

    // Call StderrPipe BEFORE Start().
    // Easy way to do it: outside the goroutine.
    errPipe, err := process.StderrPipe()
    if err != nil {
        return "", "", err
    }

    // Start process.
    if err := process.Start(); err != nil {
        return "", "", err
    }

    // Read stderr in goroutine.
    var wg sync.WaitGroup
    var stderrErr error
    wg.Add(1)
    go func() {
        defer wg.Done()
        data, err := ioutil.ReadAll(errPipe)
        if err != nil {
            stderrErr = err
        } else {
            stderr = string(data)
        }
    }()

    // Read stdout in main thread.
    data, stdoutErr := ioutil.ReadAll(outPipe)

    // Wait until we are done reading stderr.
    wg.Wait()

    // Wait for process to finish.
    if err := process.Wait(); err != nil {
        return "", "", err
    }

    // Handle error from reading stdout.
    if stdoutErr != nil {
        return "", "", stderrErr
    }
    // Handle error from reading stderr.
    if stderrErr != nil {
        return "", "", stderrErr
    }

    stdout = string(data)
    return stdout, stderr, nil
}

Much Simpler Code

All of this is done by the os/exec package automatically. You can use any io.Writer for Stdout and Stderr, you are not limited to *os.File.

func runProcess(process *exec.Cmd) (stdout, stderr string, err error) {
    var stdoutbuf, stderrbuf bytes.Buffer
    process.Stdout = &stdoutbuf
    process.Stderr = &stderrbuf
    if err := process.Run(); err != nil {
        return "", "", err
    }
    return stdoutbuf.String(), stderrbuf.String(), nil
}
user_822940
Related