occasional NPE when running in CI

[stillmatic]

Hi, I've noticed occasional (spurious) NPE in depguard when running golangci-lint in CI. We keep a go module cache between runs and speculate that unluckiness in the cache causes these errors, but we're not sure. Separately, though perhaps relatedly, we also occasionally see random packages being denylisted in CI as well, for seemingly no reason.

Here is a recent stacktrace for the NPE:

ERRO [runner] Panic: depguard: package "searchers" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference: goroutine 135617 [running]:
--
  | runtime/debug.Stack()
  | runtime/debug/stack.go:24 +0x65
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func1()
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:101 +0x155
  | panic({0xef3ac0, 0x190cf10})
  | runtime/panic.go:838 +0x207
  | sort.StringSlice.Less(...)
  | sort/sort.go:319
  | sort.doPivot({0x126a140, 0xc048a7b4a0}, 0x0, 0x428c)
  | sort/sort.go:136 +0x23c
  | sort.quickSort({0x126a140, 0xc048a7b4a0}, 0x18?, 0xec77c0?, 0xc009c26801?)
  | sort/sort.go:203 +0x6c
  | sort.Sort({0x126a140, 0xc048a7b4a0})
  | sort/sort.go:231 +0x53
  | sort.Strings(...)
  | sort/sort.go:335
  | github.com/OpenPeeDeeP/depguard.(*Depguard).initialize(0xc0026ee700, 0xc00085e3c0, 0xc0314b0a00?)
  | github.com/OpenPeeDeeP/[email protected]/depguard.go:119 +0x74
  | github.com/OpenPeeDeeP/depguard.(*Depguard).Run(0xc0026ee700, 0xc0e0b4f640?, 0x12657d0?)
  | github.com/OpenPeeDeeP/[email protected]/depguard.go:74 +0x66
  | github.com/golangci/golangci-lint/pkg/golinters.guardian.run({0xc0026ee700?, 0xc00071a510?}, 0xc009c26c78?, 0xc11fc8ddf0?, 0xc143456410)
  | github.com/golangci/golangci-lint/pkg/golinters/depguard.go:144 +0x5c
  | github.com/golangci/golangci-lint/pkg/golinters.depGuard.run({0xc00085e3c0?, {0xc00ab0d208?, 0x6000?, 0x10?}}, 0xc143456410)
  | github.com/golangci/golangci-lint/pkg/golinters/depguard.go:94 +0x3b6
  | github.com/golangci/golangci-lint/pkg/golinters.NewDepguard.func1.1(0xee2aa0?)
  | github.com/golangci/golangci-lint/pkg/golinters/depguard.go:41 +0x59
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyze(0xc01073c580)
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:187 +0x9c4
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe.func2()
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:105 +0x1d
  | github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc00358a690, {0x1026199, 0x8}, 0xc00bd37f48)
  | github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x4a
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*action).analyzeSafe(0xc0bc0a5a00?)
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_action.go:104 +0x85
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze.func2(0xc01073c580)
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:80 +0xb4
  | created by github.com/golangci/golangci-lint/pkg/golinters/goanalysis.(*loadingPackage).analyze
  | github.com/golangci/golangci-lint/pkg/golinters/goanalysis/runner_loadingpackage.go:75 +0x1eb
  | WARN [runner] Can't run linter goanalysis_metalinter: goanalysis_metalinter: depguard: package "searchers" (isInitialPkg: true, needAnalyzeSource: true): runtime error: invalid memory address or nil pointer dereference

Other stack traces have triggered for other packages than searchers eg helmtest.

Happy to provide more info.

[ldez]

hello @stillmatic, I'm interested in a reproducible use case, have you something to share?

[stillmatic]

Hey, we've tried internally and have struggled to find a setup that is consistently reproducible, unfortunately.

Locally, clearing cache seems to help, though this isn't a solution for CI - we retry lint 3x and treat it as flaky, but ofc would like not to. One observation is that these errors happen more frequently when there is an update to the go packages, but can't say anything definitively. We are also running in Go 1.18, have not tried running with 1.19.

Reading the related issues, I would say that these are likely all the same root cause. We similarly see "xyz is in the denylist" errors, those are quite a bit more frequent than this particular error.

[dixonwille]

I may have time to look at this later this week. Sorry for very late response. I used to use this all the time but recently changed employers and no longer work in this stack :( I am open to review PRs if someone is able to get it working. Otherwise please be patient with me...

[dixonwille]

Initial look, and I don't see how cache has anything to do where that stack is currently failing. The initialize phase is used turn configs into a more computer friendly format (sorted slices and compiled globs etc.). With that being said, I had a request to use go/analysis instead of the loader (which is now depricated). I am looking at including a broader feature set into it. I'll be linking a V2 issue instead here. I will probably close this out on the acceptance from golangci-lint using the v2 code instead. Hopefully resolving some of the outstanding issues along the way.

[dixonwille]

So I have a working beta v2.0.0-beta.2

go install github.com/OpenPeeDeeP/depguard/v2/cmd/[email protected]

I have tested on a few repositories. But would like others to try it out to.

[stillmatic]

thanks - I think it's easiest to reproduce/test in CI when the new release lands in golangci-lint, and it looks like the new version was picked up: golangci/golangci-lint@db4955a.

once golangci-lint releases a new version, I'll make sure to update and see if we continue to see similar behavior in CI

[dixonwille]

I have just released version 2