← Back to context

Comment by CamouflagedKiwi

8 hours ago

Nice find.

Can't help but feel this is one of the subtle traps hidden beneath the advice that contexts aren't supposed to be stored. I know it's not always that easy, of course.

5 comments

CamouflagedKiwi

Reply
compumike  7 hours ago

Thanks. I know there's a `go vet` tool that's run as part of Kubernetes CI, and one of its checks is:

  lostcancel: check cancel func returned by context.WithCancel is called

I'm not 100% sure why `go vet` didn't catch this issue, but storing the cancelFn in the struct is probably part of the reason. Any Go experts know if that's the case?

  • cyberax  6 hours ago

    The cancel function escapes the function body, so static analysis can't detect it. There's another lint for that (containedctx), but I think it's off in K8s.

    This is a serious tripping point with Go. There's no way to express: "this is a root context that I _want_ to store and only use to create derived contexts". Goroutines are also a source of problems, you can't easily say "I'm passing the ownership of this context to a goroutine".

    • compumike  6 hours ago

      It does seem like a serious tripping point.

      I took a quick look at "containedctx" and it seems like for this case, it would almost be backwards: it would flag the (not-memory-leaking) struct-stored "status.ctx", but wouldn't flag when there is a stored "status.cancelFn" only (which resulted in the memory leak).

      Could Go implement a runtime leaked-context detector, like the data race detector? https://go.dev/doc/articles/race_detector

      1 reply →