hashicorp / hashicorp/consul-template

Data race when calling Child.Stop with a process that takes too long to stop

Open
#1,753 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
4.8k
Forks
801
Avg merge
4h 5m
Merged PRs (30d)
6

Description

### Consul Template version

v0.32.0

### Configuration

The following minimal example to demonstrates the race. The full code can be found at [averche/test-go/consul-template-child-gorace](https://github.com/averche/test-go/tree/main/consul-template-child-gorace):

```go
// 0s : start the child process
// 2s : stop the child process
// 7s : the library attempts to kill the child process
func run() (int, error) {
process, err := child.New(&child.NewInput{
Command: "./my-script.sh", // a script that disregards SIGTERM
Stdout: os.Stdout,
KillSignal: syscall.SIGTERM,
KillTimeout: 5 * time.Second,
})
if err != nil {
return -2, err
}

if err := process.Start(); err != nil {
return -3, fmt.Errorf("could not start the process: %s", err)
}

select {
case <-time.After(2 * time.Second):
process.Stop()
return SuccessfullyStoppedTheProcess, nil

case exitCode := <-process.ExitCh():
return exitCode, nil
}
}

func TestRunOnce(t *testing.T) {
c, err := run()
if err != nil {
t.Fatal(err)
}
if c != SuccessfullyStoppedTheProcess {
t.Fatalf("unexpected return code: %d", c)
}
}
```

### Debug output

```sh
$ go test -run TestRunOnce --race

2023/05/25 13:55:39 [INFO] (child) spawning: ./my-script.sh
sleeping for 20s
sleeping for 19s
2023/05/25 13:55:41 [INFO] (child) stopping process
received SIGTERM; ignoring it
sleeping for 18s
sleeping for 17s
sleeping for 16s
sleeping for 15s
sleeping for 14s
==================
WARNING: DATA RACE
Write at 0x00c0000f20c0 by goroutine 6:
github.com/hashicorp/consul-template/child.(*Child).kill.func1()
/Users/avean/go/pkg/mod/github.com/hashicorp/consul-template@v0.32.0/child/child.go:439 +0x84
runtime.deferreturn()
...

Previous read at 0x00c0000f20c0 by goroutine 9:
github.com/hashicorp/consul-template/child.(*Child).kill.func2()
/Users/avean/go/pkg/mod/github.com/hashicorp/consul-template@v0.32.0/child/child.go:457 +0x70

...
```

The full output can be found @ [averche/test-go/consul-template-child-gorace](https://github.com/averche/test-go/tree/main/consul-template-child-gorace).

### Expected behavior

We attempt to stop the script, after 5 seconds, the script is killed immediately.

### Actual behavior

The script is killed correctly but a go race is detected in tests.

### Steps to reproduce

1. `git clone https://github.com/averche/test-go && cd test-go/consul-template-child-gorace/`
2. `go test -run TestRunOnce --race`
3. See the "DATA RACE" output

Contributor guide

Open the contributing guide

Research direction

Reproduce the issue with `go test -run TestRunOnce --race` from the linked consul-template-child-gorace example. Then inspect `child/child.go` around the reported lines 439 and 457 in the kill path. Done means the process is killed after the 5-second timeout and the race detector reports no data race.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
operating-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.