testing/synctest: isolation failure in encoding/json

[neild]

encoding/json uses a WaitGroup to coordinate access to a cache between multiple goroutines:
https://go.googlesource.com/go/+/refs/tags/go1.24.3/src/encoding/json/encode.go#359

When multiple synctest bubbles access the cache at the same time, a goroutine in one bubble can become blocked on a goroutine in the other bubble. Since we don't track whether the WaitGroup was created in a bubble, or which bubble it was created in, this looks like a deadlock to synctest.

A possible fix is to change encoding/json to use a sync.Once here. A goroutine blocked on a Once is not "durably" blocked in synctest terms. Also, a sync.Once is probably a clearer way of expressing the intent of this encoding/json code.

[dsnet]

FYI, the v2 implementation doesn't wait for completion for computation for the full type tree:

go/src/encoding/json/v2/arshal.go

Line 543 in 6df855e

v, _ := lookupArshalerCache.LoadOrStore(t, fncs)

It defers recursive type computation lazily with a sync.Once as suggested:

go/src/encoding/json/v2/arshal_default.go

Lines 707 to 715 in 6df855e

	var (
	once sync.Once
	keyFncs *arshaler
	valFncs *arshaler
	)
	init := func() {
	keyFncs = lookupArshaler(t.Key())
	valFncs = lookupArshaler(t.Elem())
	}

[gopherbot]

Change https://go.dev/cl/673335 mentions this issue: encoding/json: avoid supurious synctest deadlock detection

[chad-bekmezian-snap]

Isn’t this an issue that anybody using synctest could easily run into while writing completely valid code? Why wouldn’t we want to address the underlying issue, rather than the symptom?

[neild]

Why wouldn’t we want to address the underlying issue, rather than the symptom?

Do you have a specific suggestion on how we can address the underlying issue?

There's quite a bit of discussion in #67434 about the general problem of goroutines in a synctest bubble accessing global concurrency constructs. We arrived at a set of compromises which I think are, while imperfect, good enough to make the synctest package more useful than not. I don't believe there is any perfect solution to this problem, although it's probable that we can do better than the current implementation. (In particular, it's worth considering whether we can report cross-bubble use of a WaitGroup as happened here--if this had been a panic, it would still be a problem but it would have been a simpler one to diagnose.)