Data race bug #17

hansgylling · 2023-05-09T15:40:55Z

This linter has a data race bug, easily detected by installing the command with the -race flag.

$ go version
go version go1.19.9 linux/amd64
$ go install -race github.com/butuzov/mirror/cmd/mirror@latest
$ mirror ./...
==================
WARNING: DATA RACE
Write at 0x00c00013f4c8 by goroutine 9227:
  github.com/butuzov/mirror/internal/checker.(*Checker).With()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/internal/checker/checker.go:197 +0x5a9
  github.com/butuzov/mirror.(*analyzer).run.func1()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/analyzer.go:88 +0x4b7
  golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/ast/inspector/inspector.go:81 +0x116
  github.com/butuzov/mirror.(*analyzer).run()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/analyzer.go:74 +0x370
  github.com/butuzov/mirror.(*analyzer).run-fm()
      <autogenerated>:1 +0x44
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:767 +0x14c7
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce-fm()
      <autogenerated>:1 +0x39
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /usr/local/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/analysis/internal/checker.(*action).exec()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:683 +0x4c
  golang.org/x/tools/go/analysis/internal/checker.execAll.func1()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:671 +0x35
  golang.org/x/tools/go/analysis/internal/checker.execAll.func2()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x47

Previous write at 0x00c00013f4c8 by goroutine 9230:
  github.com/butuzov/mirror/internal/checker.(*Checker).With()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/internal/checker/checker.go:197 +0x5a9
  github.com/butuzov/mirror.(*analyzer).run.func1()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/analyzer.go:88 +0x4b7
  golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/ast/inspector/inspector.go:81 +0x116
  github.com/butuzov/mirror.(*analyzer).run()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/analyzer.go:74 +0x370
  github.com/butuzov/mirror.(*analyzer).run-fm()
      <autogenerated>:1 +0x44
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:767 +0x14c7
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce-fm()
      <autogenerated>:1 +0x39
  sync.(*Once).doSlow()
      /usr/local/go/src/sync/once.go:74 +0x101
  sync.(*Once).Do()
      /usr/local/go/src/sync/once.go:65 +0x46
  golang.org/x/tools/go/analysis/internal/checker.(*action).exec()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:683 +0x4c
  golang.org/x/tools/go/analysis/internal/checker.execAll.func1()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:671 +0x35
  golang.org/x/tools/go/analysis/internal/checker.execAll.func2()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x47

Goroutine 9227 (running) created at:
  golang.org/x/tools/go/analysis/internal/checker.execAll()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x22e
  golang.org/x/tools/go/analysis/internal/checker.analyze()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:311 +0x1e5
  golang.org/x/tools/go/analysis/internal/checker.Run()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:147 +0xa79
  golang.org/x/tools/go/analysis/singlechecker.Main()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/singlechecker/singlechecker.go:75 +0x34d
  main.main()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/cmd/mirror/main.go:9 +0x29

Goroutine 9230 (finished) created at:
  golang.org/x/tools/go/analysis/internal/checker.execAll()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x22e
  golang.org/x/tools/go/analysis/internal/checker.analyze()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:311 +0x1e5
  golang.org/x/tools/go/analysis/internal/checker.Run()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:147 +0xa79
  golang.org/x/tools/go/analysis/singlechecker.Main()
      /home/hansg/go/pkg/mod/golang.org/x/tools@v0.8.0/go/analysis/singlechecker/singlechecker.go:75 +0x34d
  main.main()
      /home/hansg/go/pkg/mod/github.com/butuzov/mirror@v0.1.2/cmd/mirror/main.go:9 +0x29
==================

The text was updated successfully, but these errors were encountered:

butuzov · 2023-05-10T11:14:14Z

Didn't got panic with -race flag while testing with 1.20. But i know about this "weak" point with passing additional type info to the checker. Will review it a bit later.

Juneezee · 2023-05-11T02:58:12Z

I didn't get panic with -race too.

$ go version
go version go1.20.3 linux/amd64
$ go build -race ./...

butuzov · 2023-05-11T05:47:29Z

Anyway, this is a valid issue (even if the chances of getting panic are minimal (checked with go vet, mirror itself and golangci-lint)) I am going to change the way checker works slightly so it can be avoided while working on this issue #4 as part of refactoring.

hansgylling · 2023-05-11T11:36:21Z

Didn't got panic with -race flag while testing with 1.20. But i know about this "weak" point with passing additional type info to the checker. Will review it a bit later.

Did you run the tests or the mirror command? I don't get the race while running the tests but I get it when running the command, i.e. the actual production code. This also happens with Go 1.20.4.

hansgylling · 2023-05-11T11:44:07Z

I switched to Go 1.20.4 and I'm still getting the race condition with that Go version as well. Here's a session where I'm running mirror on its own code. This time I'm building the mirror command from the main branch instead of installing it from the latest tag.

$ go version
go version go1.20.4 linux/amd64
$ cd mirror
$ git pull
Already up-to-date.
$ git status
On branch main
Your branch is up-to-date with 'origin/main'.
nothing to commit, working tree clean
$ go build -race -o mirror -trimpath cmd/mirror/main.go
$ ./mirror ./...
==================
WARNING: DATA RACE
Write at 0x00c00011f6a8 by goroutine 1162:
  github.com/butuzov/mirror/internal/checker.(*Checker).With()
      github.com/butuzov/mirror/internal/checker/checker.go:197 +0x5b6
  github.com/butuzov/mirror.(*analyzer).run.func1()
      github.com/butuzov/mirror/analyzer.go:86 +0x4c2
  golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder()
      golang.org/x/tools@v0.8.0/go/ast/inspector/inspector.go:81 +0x111
  github.com/butuzov/mirror.(*analyzer).run()
      github.com/butuzov/mirror/analyzer.go:72 +0x370
  github.com/butuzov/mirror.(*analyzer).run-fm()
      <autogenerated>:1 +0x44
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:767 +0x14c7
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce-fm()
      <autogenerated>:1 +0x39
  sync.(*Once).doSlow()
      sync/once.go:74 +0x101
  sync.(*Once).Do()
      sync/once.go:65 +0x46
  golang.org/x/tools/go/analysis/internal/checker.(*action).exec()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:683 +0x4c
  golang.org/x/tools/go/analysis/internal/checker.execAll.func1()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:671 +0x35
  golang.org/x/tools/go/analysis/internal/checker.execAll.func2()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x47

Previous write at 0x00c00011f6a8 by goroutine 1165:
  github.com/butuzov/mirror/internal/checker.(*Checker).With()
      github.com/butuzov/mirror/internal/checker/checker.go:197 +0x5b6
  github.com/butuzov/mirror.(*analyzer).run.func1()
      github.com/butuzov/mirror/analyzer.go:86 +0x4c2
  golang.org/x/tools/go/ast/inspector.(*Inspector).Preorder()
      golang.org/x/tools@v0.8.0/go/ast/inspector/inspector.go:81 +0x111
  github.com/butuzov/mirror.(*analyzer).run()
      github.com/butuzov/mirror/analyzer.go:72 +0x370
  github.com/butuzov/mirror.(*analyzer).run-fm()
      <autogenerated>:1 +0x44
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:767 +0x14c7
  golang.org/x/tools/go/analysis/internal/checker.(*action).execOnce-fm()
      <autogenerated>:1 +0x39
  sync.(*Once).doSlow()
      sync/once.go:74 +0x101
  sync.(*Once).Do()
      sync/once.go:65 +0x46
  golang.org/x/tools/go/analysis/internal/checker.(*action).exec()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:683 +0x4c
  golang.org/x/tools/go/analysis/internal/checker.execAll.func1()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:671 +0x35
  golang.org/x/tools/go/analysis/internal/checker.execAll.func2()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x47

Goroutine 1162 (running) created at:
  golang.org/x/tools/go/analysis/internal/checker.execAll()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x22e
  golang.org/x/tools/go/analysis/internal/checker.analyze()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:311 +0x1e5
  golang.org/x/tools/go/analysis/internal/checker.Run()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:147 +0xa84
  golang.org/x/tools/go/analysis/singlechecker.Main()
      golang.org/x/tools@v0.8.0/go/analysis/singlechecker/singlechecker.go:75 +0x34d
  main.main()
      main.go:9 +0x29

Goroutine 1165 (finished) created at:
  golang.org/x/tools/go/analysis/internal/checker.execAll()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:677 +0x22e
  golang.org/x/tools/go/analysis/internal/checker.analyze()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:311 +0x1e5
  golang.org/x/tools/go/analysis/internal/checker.Run()
      golang.org/x/tools@v0.8.0/go/analysis/internal/checker/checker.go:147 +0xa84
  golang.org/x/tools/go/analysis/singlechecker.Main()
      golang.org/x/tools@v0.8.0/go/analysis/singlechecker/singlechecker.go:75 +0x34d
  main.main()
      main.go:9 +0x29
==================

butuzov · 2023-05-11T12:19:24Z

Let me come back with a bit refactored version, ok?

hansgylling · 2023-05-11T12:25:02Z

Sure, no stress from me. I just wanted to show that I can reproduce the issue with Go 1.20 since there were two people saying they couldn't. I'm glad you're looking into it and reply fast. Not every open source project where you get that kind of response. I hope I can use mirror as part of golangci-lint soon.

butuzov · 2023-05-11T14:11:07Z

sidenote: feeling weird, considering I am now 2km from the zero line. calm day, with no shelling, has an LTE signal and can focus.

butuzov · 2023-05-12T10:33:06Z

hi, @hansgylling can you try the current head?

hansgylling · 2023-05-12T12:59:32Z

Yes, I have now tried the latest commit on main many times on several code bases and I can't reproduce the data race anymore. I say this bug is resolved.

hansgylling mentioned this issue May 9, 2023

mirror: linter that suggest using alternative string/[]byte functions golangci/golangci-lint#3825

Merged

hansgylling closed this as completed May 12, 2023

hansgylling mentioned this issue Oct 17, 2023

Data race in ireturn butuzov/ireturn#63

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data race bug #17

Data race bug #17

hansgylling commented May 9, 2023

butuzov commented May 10, 2023 •

edited

Loading

Juneezee commented May 11, 2023 •

edited

Loading

butuzov commented May 11, 2023

hansgylling commented May 11, 2023 •

edited

Loading

hansgylling commented May 11, 2023 •

edited

Loading

butuzov commented May 11, 2023

hansgylling commented May 11, 2023

butuzov commented May 11, 2023 •

edited

Loading

butuzov commented May 12, 2023

hansgylling commented May 12, 2023

Data race bug #17

Data race bug #17

Comments

hansgylling commented May 9, 2023

butuzov commented May 10, 2023 • edited Loading

Juneezee commented May 11, 2023 • edited Loading

butuzov commented May 11, 2023

hansgylling commented May 11, 2023 • edited Loading

hansgylling commented May 11, 2023 • edited Loading

butuzov commented May 11, 2023

hansgylling commented May 11, 2023

butuzov commented May 11, 2023 • edited Loading

butuzov commented May 12, 2023

hansgylling commented May 12, 2023

butuzov commented May 10, 2023 •

edited

Loading

Juneezee commented May 11, 2023 •

edited

Loading

hansgylling commented May 11, 2023 •

edited

Loading

hansgylling commented May 11, 2023 •

edited

Loading

butuzov commented May 11, 2023 •

edited

Loading