Skip to content
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

Add mutex to ensure struct gets initialized once #23

Merged
merged 1 commit into from Aug 30, 2022
Merged

Add mutex to ensure struct gets initialized once #23

merged 1 commit into from Aug 30, 2022

Conversation

Bryan1041
Copy link

Running depguard via golangci-lint run on a large number of packages (e.g. over 600) sometimes results in "is not in the allowlist" errors for packages that are in the allowlist.

It looks like Run/initialize is being called by multiple goroutines on a shared struct in golangci-lint, so prefixPackages and possibly other struct members are ending up with duplicate, missing and/or incomplete package names.

This change ensures that the struct gets initialized once on the first call.

@dixonwille
Copy link
Member

I'll take a look at this. Curious what performance hit this may incur. Was this a change golangci lint made recently that causes it to call a linter's initializer more than once? I don't recall this being the case when this was initially developed.

@dixonwille
Copy link
Member

Thanks for the PR. I am concerned about performance, but as I am working on a V2 for depguard, this shouldn't be an issue moving forward (using go/analysis which is structured to be concurrent access I believe). Also poor planning on my part for initializing during run instead of inside a NewFunc() that handles it before the execution...

@dixonwille dixonwille merged commit 27892bc into OpenPeeDeeP:master Aug 30, 2022
@Bryan1041
Copy link
Author

@dixonwille Thanks for the review (and the linter). It's been a while since I looked at it, but I don't remember seeing any changes to how plugins are initialized in golangci-lint. I will say that when I tested this, the likelihood of a false positive flag increased with the number of packages being linted and seems to be pretty rare unless you are linting a lot of packages.

Can we create a tag for this? If so, I will open a PR to upgrade the dependency in golangci-lint as a temporary fix until V2.

@dixonwille
Copy link
Member

@Bryan1041 just tagged v1.1.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants