pb33f / doctor

The Doctor. Just the doctor, no more, no less.
https://pb33f.io
Other
4 stars 3 forks source link

Doctor Will Trigger a Race Condition with Build Graph Enabled #6

Open califlower opened 2 weeks ago

califlower commented 2 weeks ago

Running doctor with build graph enabled triggers a race condition. Can be seen using vacuum with BuildGraph On.

WARNING: DATA RACE
Read at 0x00c00012c7c0 by goroutine 317:
  runtime.growslice()
      /nix/store/4cijk6gwv59c84h1l9yhxzsaz93f67mz-go-1.23.0/share/go/src/runtime/slice.go:177 +0x0
  github.com/pb33f/doctor/model/high/base.(*Foundation).AddEdge()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/base/foundation.go:316 +0x94
  github.com/pb33f/doctor/model/high/v3.(*PathItem).AddEdge()
      <autogenerated>:1 +0x28
  github.com/pb33f/doctor/model/high/base.(*Foundation).BuildNodesAndEdges()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/base/foundation.go:174 +0x800
  github.com/pb33f/doctor/model/high/v3.(*Operation).Walk()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/operation.go:33 +0x130
  github.com/pb33f/doctor/model/high/v3.(*PathItem).Walk.func1()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/path_item.go:81 +0x64
  github.com/sourcegraph/conc/panics.(*Catcher).Try()
      <redacted>/vendor/github.com/sourcegraph/conc/panics/panics.go:23 +0x70
  github.com/sourcegraph/conc.(*WaitGroup).Go.func1()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:32 +0x84

Previous write at 0x00c00012c7c0 by goroutine 318:
  github.com/pb33f/doctor/model/high/base.(*Foundation).AddEdge()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/base/foundation.go:314 +0x154
  github.com/pb33f/doctor/model/high/v3.(*PathItem).AddEdge()
      <autogenerated>:1 +0x28
  github.com/pb33f/doctor/model/high/base.(*Foundation).BuildNodesAndEdges()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/base/foundation.go:174 +0x800
  github.com/pb33f/doctor/model/high/v3.(*Server).Walk()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/server.go:26 +0x168
  github.com/pb33f/doctor/model/high/v3.(*PathItem).Walk.func9()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/path_item.go:138 +0x50
  github.com/sourcegraph/conc/panics.(*Catcher).Try()
      <redacted>/vendor/github.com/sourcegraph/conc/panics/panics.go:23 +0x70
  github.com/sourcegraph/conc.(*WaitGroup).Go.func1()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:32 +0x84

Goroutine 317 (running) created at:
  github.com/sourcegraph/conc.(*WaitGroup).Go()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:30 +0xd4
  github.com/pb33f/doctor/model/high/v3.(*PathItem).Walk()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/path_item.go:81 +0x234
  github.com/pb33f/doctor/model/high/v3.(*Paths).Walk.func1()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/paths.go:47 +0x50
  github.com/sourcegraph/conc/panics.(*Catcher).Try()
      <redacted>/vendor/github.com/sourcegraph/conc/panics/panics.go:23 +0x70
  github.com/sourcegraph/conc.(*WaitGroup).Go.func1()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:32 +0x84

Goroutine 318 (running) created at:
  github.com/sourcegraph/conc.(*WaitGroup).Go()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:30 +0xd4
  github.com/pb33f/doctor/model/high/v3.(*PathItem).Walk()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/path_item.go:138 +0xe2c
  github.com/pb33f/doctor/model/high/v3.(*Paths).Walk.func1()
      <redacted>/vendor/github.com/pb33f/doctor/model/high/v3/paths.go:47 +0x50
  github.com/sourcegraph/conc/panics.(*Catcher).Try()
      <redacted>/vendor/github.com/sourcegraph/conc/panics/panics.go:23 +0x70
  github.com/sourcegraph/conc.(*WaitGroup).Go.func1()
      <redacted>/vendor/github.com/sourcegraph/conc/waitgroup.go:32 +0x84

The reason is https://github.com/pb33f/doctor/blob/main/model/high/base/foundation.go#L306

      func (f *Foundation) AddEdge(edge *Edge) {
    if f == nil {
        return
    }
    if edge == nil {
        return
    }
    if f.Edges == nil {
        f.Edges = []*Edge{edge}
    } else {
        f.Edges = append(f.Edges, edge)
    }
}

does not take a lock on appending to f.Edges using the built in struct Mutex.