Skip to content

Command timeout not triggered for interactive shell command. #29173

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
zbindenren opened this issue Dec 11, 2018 · 1 comment
Closed

Command timeout not triggered for interactive shell command. #29173

zbindenren opened this issue Dec 11, 2018 · 1 comment

Comments

@zbindenren
Copy link

zbindenren commented Dec 11, 2018

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://[email protected]/kelseyhightower/doesnotexist.git
        // in order the git command asks for password
        modifiedConfig := bytes.Replace(config, []byte("https://github.com/kelseyhightower/nocode.git"), []byte("https://[email protected]/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://[email protected]': panic: test timed out after 10s

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

The second test TestRun works as expected:

Password for 'https://[email protected]': 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
Copy link
Contributor

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.

@golang golang locked and limited conversation to collaborators Dec 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants