Doctor Will Trigger a Race Condition with Build Graph Enabled
Closed this issue · 4 comments
califlower commented
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
.
daveshanley commented
Thank you!
daveshanley commented
I have added a mutex here. Can you re-try?
califlower commented
@daveshanley yes sure! Let me take a look
califlower commented
I no longer get this, closing the issue. Thank you!