go test -race ./... fails in two packages. One is a test bug; the other is in
product code and is the one that matters.
Found on the v0.0.6 tag run, before the race job was moved to dispatch-only.
1. internal/app/session — product code
WARNING: DATA RACE
Write at 0x00c00015b760 by goroutine 3835:
session.(*Sim).buildSeeded() internal/app/session/engine.go:393
session.(*Sim).build() internal/app/session/engine.go:276
session.Register.func3() internal/app/session/verbs.go:169
state.(*Store).Run() internal/app/state/store.go:160
Previous read at 0x00c00015b760 by goroutine 3836:
session.(*Sim).Close() internal/app/session/enginefirmware.go:126
session.registerUI.func3.gowrap2() internal/app/session/ui.go:147
--- FAIL: TestVerbsAreSafeBeforeAnythingIsLoaded
buildSeeded assigns s.eng = engine.New(...) on the store goroutine,
while Close reads if s.eng == nil on the workbench's shutdown
goroutine. Unsynchronised, on the same field.
What makes this worth reading rather than just fixing: the comment directly
above Close already knows it is called from another goroutine, and takes
care over exactly one field —
Called from the workbench's own shutdown goroutine rather than the store's,
so the served listeners are taken out under their lock and closed only once
it is released - a verb still running on the store's goroutine at the same
moment must never see a map mid-edit.
served is guarded. s.eng, read two lines later in the same function, is
not. The reasoning was right and was applied to one field of the pair.
Consequences are worse than a failed test: a close racing a build can read a
half-assigned s.eng, or read nil and skip the teardown entirely, leaking
the engine and its listeners.
2. internal/fetchasset_test.go — test only
WARNING: DATA RACE
Read at ... by goroutine 16: net/http.serverHandler.ServeHTTP()
Previous write at ... by goroutine 12:
internal_test.stubAPI() internal/fetchasset_test.go:69
--- FAIL: TestFetchReleaseAsset
--- FAIL: TestFetchEmulatorTellsTheTwoStatesApart
stubAPI does httptest.NewServer(nil), which starts serving
immediately, then assigns srv.Config.Handler = mux at line 69. The serve
loop reads Handler while the test writes it.
The shape is inherited from the handlers needing srv.URL, which does not
exist until the server is up. httptest.NewUnstartedServer(nil) fixes it
without losing that: build the mux (the closures read srv.URL at request
time, not at construction), assign srv.Config.Handler, then srv.Start().
Why this stood
The race job only ran on release tags, and a red detector never blocked
anything — package.yml is a separate workflow, so it sat red without
stopping a release. It is dispatch-only now, which makes running it a
deliberate act rather than noise nobody acts on. That change makes this issue
the record instead.
Reproduce: go test -race -count=1 ./internal/ ./internal/app/session/
go test -race ./...fails in two packages. One is a test bug; the other is inproduct code and is the one that matters.
Found on the
v0.0.6tag run, before the race job was moved to dispatch-only.1.
internal/app/session— product codebuildSeededassignss.eng = engine.New(...)on the store goroutine,while
Closereadsif s.eng == nilon the workbench's shutdowngoroutine. Unsynchronised, on the same field.
What makes this worth reading rather than just fixing: the comment directly
above
Closealready knows it is called from another goroutine, and takescare over exactly one field —
servedis guarded.s.eng, read two lines later in the same function, isnot. The reasoning was right and was applied to one field of the pair.
Consequences are worse than a failed test: a close racing a build can read a
half-assigned
s.eng, or readniland skip the teardown entirely, leakingthe engine and its listeners.
2.
internal/fetchasset_test.go— test onlystubAPIdoeshttptest.NewServer(nil), which starts servingimmediately, then assigns
srv.Config.Handler = muxat line 69. The serveloop reads
Handlerwhile the test writes it.The shape is inherited from the handlers needing
srv.URL, which does notexist until the server is up.
httptest.NewUnstartedServer(nil)fixes itwithout losing that: build the mux (the closures read
srv.URLat requesttime, not at construction), assign
srv.Config.Handler, thensrv.Start().Why this stood
The race job only ran on release tags, and a red detector never blocked
anything —
package.ymlis a separate workflow, so it sat red withoutstopping a release. It is dispatch-only now, which makes running it a
deliberate act rather than noise nobody acts on. That change makes this issue
the record instead.
Reproduce:
go test -race -count=1 ./internal/ ./internal/app/session/