golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.42k stars 17.71k forks source link

Command timeout not triggered for interactive shell command. #29173

Closed zbindenren closed 5 years ago

zbindenren commented 5 years ago

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

$ go version
go version go1.11.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

go env Output
$ go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/rz/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/rz/golang"
GOPROXY=""
GORACE=""
GOROOT="/home/rz/.gimme/versions/go1.11.2.linux.amd64"
GOTMPDIR=""
GOTOOLDIR="/home/rz/.gimme/versions/go1.11.2.linux.amd64/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/rz/golang/src/git.pnet.ch/golang/pollout/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build654284275=/tmp/go-build -gno-record-gcc-switches"

What did you do?

For the following test:

package main

import (
        "bytes"
        "context"
        "io/ioutil"
        "os"
        "os/exec"
        "path"
        "testing"
        "time"
)

func TestOutput(t *testing.T) {
        const repoDest = "/tmp/nocode"
        if err := setup(repoDest); err != nil {
                t.Fatal(err)
        }
        t.Log("setup complete")
        ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
        defer cancel()
        cmd := exec.CommandContext(ctx, "git", "-C", repoDest, "remote", "update", "--prune")
        if _, err := cmd.CombinedOutput(); err != nil {
                t.Error("error is nil but should not")
        }
}

func TestRun(t *testing.T) {
        const repoDest = "/tmp/nocode"
        if err := setup(repoDest); err != nil {
                t.Fatal(err)
        }
        t.Log("setup complete")
        ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
        defer cancel()
        cmd := exec.CommandContext(ctx, "git", "-C", repoDest, "remote", "update", "--prune")
        if err := cmd.Run(); err == nil {
                t.Error("error is nil but should not")
        }
}

func setup(repoDest string) error {
        if err := os.RemoveAll("/tmp/nocode"); err != nil {
                return err
        }
        cmd := exec.Command("git", "clone", "--bare", "https://github.com/kelseyhightower/nocode.git", repoDest)
        if err := cmd.Run(); err != nil {
                return err
        }

        config, err := ioutil.ReadFile(path.Join(repoDest, "config"))
        if err != nil {
                return err
        }
        // changing https://github.com/kelseyhightower/nocode.git to https://user@github.com/kelseyhightower/doesnotexist.git
        // in order the git command asks for password
        modifiedConfig := bytes.Replace(config, []byte("https://github.com/kelseyhightower/nocode.git"), []byte("https://user@github.com/kelseyhightower/doesnotexist.git"), 1)

        if err := ioutil.WriteFile(path.Join(repoDest, "config"), modifiedConfig, 0644); err != nil {
                return err
        }
        return nil
}

The first test TestOutput runs into the test timeout and context timeout is not triggered:

go test -timeout 10s -run Output
Password for 'https://user@github.com': panic: test timed out after 10s

goroutine 7 [running]:
testing.(*M).startAlarm.func1()
...

The second test TestRun works as expected:

Password for 'https://user@github.com': PASS
ok      git.pnet.ch/golang/pollout      3.139s

What did you expect to see?

That for both tests the context timeout of 2 seconds is triggered.

What did you see instead?

Only the second test behaves as expected.

ianlancetaylor commented 5 years ago

This is another version of #23019. git is starting a subprocess to read the password, the context timeout only kills the parent process, and the use of CombinedOutput means that the main program hangs waiting for the subprocess to terminate. There is no simple fix here. You can arrange for your main program to not hang by not calling CombinedOutput and using pipes instead, but that won't kill the subprocess.

I'm going to close this as a dup of #23019.