runtime: lock ordering problem between trace and wbufSpans

[bcmills]

#!watchflakes
post <- `fatal error: lock ordering problem` && `0 : trace` && `1 : wbufSpans`

https://build.golang.org/log/af083d40156b011db2e0cadd6040692cf579967c:

99928  ======
0 : trace 42 0x657b20
1 : wbufSpans 39 0x651030
fatal error: lock ordering problem

runtime stack:
runtime.throw({0x553dbd?, 0x0?})
	/workdir/go/src/runtime/panic.go:1047 +0x5d fp=0x7f153fffeb58 sp=0x7f153fffeb28 pc=0x4392fd
runtime.checkRanks(0xc0001021a0, 0xc000067000?, 0x4cb503?)
	/workdir/go/src/runtime/lockrank_on.go:156 +0x22d fp=0x7f153fffebb8 sp=0x7f153fffeb58 pc=0x40de8d
runtime.lockWithRankMayAcquire.func1()
	/workdir/go/src/runtime/lockrank_on.go:229 +0x89 fp=0x7f153fffebe8 sp=0x7f153fffebb8 pc=0x40e5c9
runtime.lockWithRankMayAcquire(0x40e0cc?, 0xc0001021a0?)
	/workdir/go/src/runtime/lockrank_on.go:218 +0x65 fp=0x7f153fffec20 sp=0x7f153fffebe8 pc=0x40e505
runtime.getempty()
	/workdir/go/src/runtime/mgcwork.go:362 +0x3f fp=0x7f153fffec80 sp=0x7f153fffec20 pc=0x42881f
runtime.(*gcWork).init(0xc000030638)
	/workdir/go/src/runtime/mgcwork.go:101 +0x1c fp=0x7f153fffec90 sp=0x7f153fffec80 pc=0x4280dc
runtime.(*gcWork).putBatch(0xc000030638, {0xc000030670, 0x1, 0x200})
	/workdir/go/src/runtime/mgcwork.go:175 +0x46 fp=0x7f153fffece8 sp=0x7f153fffec90 pc=0x4282c6
runtime.wbBufFlush1(0xc00002f400)
	/workdir/go/src/runtime/mwbbuf.go:287 +0x24f fp=0x7f153fffed30 sp=0x7f153fffece8 pc=0x4348ef
runtime.wbBufFlush.func1()
	/workdir/go/src/runtime/mwbbuf.go:201 +0x25 fp=0x7f153fffed48 sp=0x7f153fffed30 pc=0x4671c5
runtime.wbBufFlush(0x40e280?, 0xc0001021a0?)
	/workdir/go/src/runtime/mwbbuf.go:200 +0x6c fp=0x7f153fffed68 sp=0x7f153fffed48 pc=0x43464c
runtime.wbBufFlush(0x657b38, 0x0)
	<autogenerated>:1 +0x2a fp=0x7f153fffed88 sp=0x7f153fffed68 pc=0x47052a
runtime.gcWriteBarrier()
	/workdir/go/src/runtime/asm_amd64.s:1692 +0xa3 fp=0x7f153fffee08 sp=0x7f153fffed88 pc=0x46dfc3
runtime.gcWriteBarrierDX()
	/workdir/go/src/runtime/asm_amd64.s:1720 +0x7 fp=0x7f153fffee10 sp=0x7f153fffee08 pc=0x46e047
runtime.readTrace0()
	/workdir/go/src/runtime/trace.go:535 +0x49a fp=0x7f153fffee50 sp=0x7f153fffee10 pc=0x45c59a
runtime.ReadTrace.func1()
	/workdir/go/src/runtime/trace.go:439 +0x2a fp=0x7f153fffee70 sp=0x7f153fffee50 pc=0x45c0aa
runtime.systemstack()
	/workdir/go/src/runtime/asm_amd64.s:492 +0x49 fp=0x7f153fffee78 sp=0x7f153fffee70 pc=0x46bd29

goroutine 188 [running]:
runtime.systemstack_switch()
	/workdir/go/src/runtime/asm_amd64.s:459 fp=0xc0001f2738 sp=0xc0001f2730 pc=0x46bcc0
runtime.ReadTrace()
	/workdir/go/src/runtime/trace.go:438 +0x88 fp=0xc0001f27a0 sp=0xc0001f2738 pc=0x45c028
runtime/trace.Start.func1()
	/workdir/go/src/runtime/trace/trace.go:130 +0x47 fp=0xc0001f27e0 sp=0xc0001f27a0 pc=0x4beb67
runtime.goexit()
	/workdir/go/src/runtime/asm_amd64.s:1594 +0x1 fp=0xc0001f27e8 sp=0xc0001f27e0 pc=0x46dee1
created by runtime/trace.Start
	/workdir/go/src/runtime/trace/trace.go:128 +0xef

(attn @golang/runtime)

[mknyszek]

I still plan to look into this.

[mknyszek]

Oh no. This is real. You can't actually have a write barrier while holding trace.lock. Thing is, I don't see where there would be a write barrier by just looking at the source.

[cherrymui]

It seems the write barrier is https://cs.opensource.google/go/go/+/release-branch.go1.20:src/runtime/trace.go;l=535 (Note: I used 1.20 release branch to point to old code). That code is gone with your CL https://golang.org/cl/496296 . So it is probably fixed.

[mknyszek]

Thanks for digging it up. I tried the 1.20 release branch but couldn't find it; probably a misclick and I was on the wrong branch in the end.

Anyway, yes! Indeed, looks like this problem is solved on tip.

[gopherbot]

Found new dashboard test flakes for:

#!watchflakes
post <- `fatal error: lock ordering problem` && `0 : trace` && `1 : wbufSpans`

2023-10-25 19:47 linux-amd64-staticlockranking go@a57c5736 runtime/trace.TestTraceFutileWakeup (log)

84254  ======
0 : traceStackTab 43 0x7265e0
1 : wbufSpans 38 0x71f0b0
fatal error: lock ordering problem

runtime stack:
runtime.throw({0x5ce82a?, 0x0?})
	/workdir/go/src/runtime/panic.go:1016 +0x5c fp=0x7fd1da7fb850 sp=0x7fd1da7fb820 pc=0x43b4bc
runtime.checkRanks(0xc000105520, 0x0?, 0x0?)
	/workdir/go/src/runtime/lockrank_on.go:162 +0x236 fp=0x7fd1da7fb8b0 sp=0x7fd1da7fb850 pc=0x40e7b6
...
	/workdir/go/src/runtime/proc.go:406
runtime.semacquire1(0xc0000781e8, 0x0, 0x1, 0x0, 0x12)
	/workdir/go/src/runtime/sema.go:160 +0x22f fp=0xc000067d60 sp=0xc000067d00 pc=0x44fbcf
sync.runtime_Semacquire(0xc00008c060?)
	/workdir/go/src/runtime/sema.go:62 +0x25 fp=0xc000067d98 sp=0xc000067d60 pc=0x46c4e5
sync.(*WaitGroup).Wait(0x8?)
	/workdir/go/src/sync/waitgroup.go:116 +0x48 fp=0xc000067dc0 sp=0xc000067d98 pc=0x4782c8
runtime/trace_test.TestTraceFutileWakeup(0xc000324680)
	/workdir/go/src/runtime/trace/trace_test.go:558 +0x372 fp=0xc000067f70 sp=0xc000067dc0 pc=0x574e92
testing.tRunner(0xc000324680, 0x5d9648)

— watchflakes

[mknyszek]

I think this is fixed as of the new tracer, and a bunch of the recent staticlockranking fixes for it.