func requestFromSlowServer(ctx context.Context) string {
select {
case <-ctx.Done():
return ctx.Err().Error()
case <-time.After(time.Second):
}
return "very important data"
}
func requestData(timeout time.Duration) string {
dataChan := make(chan string)
go func() {
ctx := context.Background()
newData := requestFromSlowServer(ctx)
select {
case <-ctx.Done():
case dataChan <- newData:
}
}()
select {
case result := <- dataChan:
fmt.Printf("[+] request returned: %s", result)
return result
case <- time.After(timeout):
// Hitting this case doesn't cancel the context, so you're still leaking goroutines.
fmt.Println("[!] request timeout!")
return ""
}
}
You can try this example in their playground (https://semgrep.dev/s/Q8Bo/) and note that it basically does the exact same thing as before (context.Background() never gates the channel sends or receives, but it does convince the linter that you aren't leaking). I got here by using context.WithTimeout(...) first, which was correctly marked as not leaking. But it's faked out by any gating, not by actually useful gating. So it just pushes the problem to more subtle cases that are even easier for code reviewers to miss. "foo <- bar" is always something a code reviewer is going to worry about, but creating a context ten levels up and still leaking goroutines is much more subtle and would be just the thing that static analysis could make the reviewer aware of. (Basically saying, "hey, you did this right, but there are some implementation details that prevent it from working".)Faking out the linter easily is pretty scary. I ran into a big memory leak with Sentry's client library where they weren't closing http.Response.Body. They had a lint rule to make sure they were closing the body, but it didn't fire because they managed to fake out the linter (by passing the response to a helper function, which didn't close the body).
Overall I think this is too simplistic to benefit from unless you have found a particular pattern used by a particular author in a particular codebase, and want to get a list of places to fix. Certainly, a lot of new programmers to Go forget that channel writes block, and don't gate the write with a timeout condition. But fixing this is kind of the second thing you learn in Go ;)
I thought "do I need to integrate semgrep with my CI", and was underwhelmed by the default Go ruleset: https://semgrep.dev/p/golang. Making sure that people don't connect to SSH without verifying the server's host key is not in my top 15 lint rules (no complaints about including that, but it's very situational), but that's the sort of thing they provide out of the box. This particular rule doesn't even make it into their default list of rules.