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

Fix panic in filecache #594

Merged
merged 2 commits into from Jul 14, 2019
Merged

Fix panic in filecache #594

merged 2 commits into from Jul 14, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jul 1, 2019

Hi!

We've got panic happened while liniting our repository with golangci-lint. The reason is concurrent writes in filecache map. I tried to fix it via using sync.Map instead of map, so there must be no concurrent [read/]writes. Also there might be possible race in linecache because there is no any mutex either.

Panic
if [ "Darwin" = "Darwin" ]; then export PKG_CONFIG_PATH=/opt/X11/lib/pkgconfig/; else export PKG_CONFIG_PATH=/usr/local/lib/pkgconfig/; fi && \
	golangci-lint run -c golangci_extended.yml -v  ./src/mail/...
level=info msg="[config_reader] Used config file golangci_extended.yml"
level=info msg="[lintersdb] Active 29 linters: [bodyclose deadcode depguard dupl errcheck goconst gocritic gocyclo gofmt goimports golint gosec gosimple govet ineffassign interfacer lll misspell nakedret prealloc scopelint staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck]"
level=info msg="[lintersdb] Optimized sublinters [staticcheck gosimple unused stylecheck] into metalinter megacheck"
level=info msg="[loader] Go packages loading at mode load types and syntax took 19.223275966s"
level=info msg="[loader] SSA repr building timing: packages building 266.546786ms, total 6.847481739s"
src/mail/api/clickerproxy/vid.go:59:42: bodyclose: response body must be closed (bodyclose)
	resp, err := requests.BaseOnce().Request(ctx, req, requests.Params{
	                                        ^
fatal error: concurrent map writes

goroutine 10801 [running]:
runtime.throw(0x1a8048f, 0x15)
	/usr/local/go/src/runtime/panic.go:617 +0x72 fp=0xc0020916a0 sp=0xc002091670 pc=0x102e862
runtime.mapassign_faststr(0x18efd40, 0xc000468420, 0xc000517560, 0x51, 0x1742)
	/usr/local/go/src/runtime/map_faststr.go:211 +0x42a fp=0xc002091708 sp=0xc0020916a0 pc=0x1014e5a
github.com/golangci/golangci-lint/pkg/fsutils.(*FileCache).GetFileBytes(0xc00046e0c8, 0xc000517560, 0x51, 0xc166d77500, 0x991, 0xc034f46c80, 0x0, 0x8)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/fsutils/filecache.go:33 +0x10b fp=0xc002091798 sp=0xc002091708 pc=0x140ce4b
github.com/golangci/golangci-lint/pkg/golinters.Misspell.runOnFile(0xc000517560, 0x51, 0xc002091b18, 0xc1644e5f00, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/golinters/misspell.go:68 +0x61 fp=0xc0020919f8 sp=0xc002091798 pc=0x17e3311
github.com/golangci/golangci-lint/pkg/golinters.Misspell.Run(0x1bde7c0, 0xc000032f60, 0xc1644e5f00, 0xc03caf4860, 0x1, 0x1, 0x1bacdd4, 0xc000054990)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/golinters/misspell.go:56 +0x1cc fp=0xc002091b58 sp=0xc0020919f8 pc=0x17e2b7c
github.com/golangci/golangci-lint/pkg/golinters.(*Misspell).Run(0x24136c8, 0x1bde7c0, 0xc000032f60, 0xc1644e5f00, 0xc120e73e90, 0x203048, 0x10228d5, 0x0, 0x1870ebcb)
	<autogenerated>:1 +0x58 fp=0xc002091ba8 sp=0xc002091b58 pc=0x17ea758
github.com/golangci/golangci-lint/pkg/lint.(*Runner).runLinterSafe(0xc002091ee0, 0x1bde7c0, 0xc000032f60, 0xc03cb06000, 0xc0003d1d00, 0x0, 0x0, 0x0, 0x0, 0x0)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:112 +0x16a fp=0xc002091ca8 sp=0xc002091ba8 pc=0x180bc4a
github.com/golangci/golangci-lint/pkg/lint.Runner.runWorker.func1()
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:146 +0x64 fp=0xc002091d18 sp=0xc002091ca8 pc=0x180d924
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0xc03bea4280, 0x1a129c6, 0x8, 0xc002091e40)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:75 +0x50 fp=0xc002091d78 sp=0xc002091d18 pc=0x1806d40
github.com/golangci/golangci-lint/pkg/lint.Runner.runWorker(0xc03c910000, 0x11, 0x11, 0x1be8880, 0xc03ca64c90, 0x1bde7c0, 0xc000032f60, 0xc03cb06000, 0xc03caeea80, 0xc03caeec00, ...)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:145 +0x23b fp=0xc002091ee0 sp=0xc002091d78 pc=0x180c00b
github.com/golangci/golangci-lint/pkg/lint.(*Runner).runWorkers.func1(0xc02c718490, 0xc03ca30270, 0x1bde7c0, 0xc000032f60, 0xc03cb06000, 0xc03caeea80, 0xc03caeec00, 0xc03bb96480, 0x8, 0x8, ...)
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:200 +0x163 fp=0xc002091f88 sp=0xc002091ee0 pc=0x180db53
runtime.goexit()
	/usr/local/go/src/runtime/asm_amd64.s:1337 +0x1 fp=0xc002091f90 sp=0xc002091f88 pc=0x105d091
created by github.com/golangci/golangci-lint/pkg/lint.(*Runner).runWorkers
	/Users/ndr/go/src/gitlab.corp.mail.ru/e-mail-ru/mailapi/vendor/src/github.com/golangci/golangci-lint/pkg/lint/runner.go:197 +0x1ab

Full output of this run is here

https://gist.github.com/be-b10g/6be369a6ff3a237496c6fca004487566

@CLAassistant
Copy link

CLAassistant commented Jul 1, 2019

CLA assistant check
All committers have signed the CLA.

@jirfag jirfag merged commit e175825 into golangci:master Jul 14, 2019
@jirfag
Copy link
Member

jirfag commented Jul 14, 2019

thank you very much for the fix!

@jirfag
Copy link
Member

jirfag commented Jul 14, 2019

Also, I will try to enable tests with -race for master branch builds (too slow for regular branch builds)

@Helcaraxan Helcaraxan mentioned this pull request Aug 29, 2019
@ldez ldez added this to the v1.18 milestone Mar 6, 2024
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.

4 participants