From 342db2851becda8a67b278206507878839130788 Mon Sep 17 00:00:00 2001 From: Thirathawat Chanserikorn Date: Fri, 5 Jun 2026 18:55:13 +0700 Subject: [PATCH] fix: make decoder cache publication atomic --- decode_test.go | 47 ++++++++++++ internal/decoder/compile.go | 24 ++++++- internal/decoder/compile_norace.go | 5 +- internal/decoder/compile_race.go | 13 +--- internal/decoder/compile_test.go | 111 +++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+), 16 deletions(-) create mode 100644 internal/decoder/compile_test.go diff --git a/decode_test.go b/decode_test.go index 8f32cedb..baf47c99 100644 --- a/decode_test.go +++ b/decode_test.go @@ -14,6 +14,7 @@ import ( "reflect" "strconv" "strings" + "sync" "testing" "time" "unsafe" @@ -4057,3 +4058,49 @@ func TestIssue429(t *testing.T) { } } } + +func TestIssue495(t *testing.T) { + type issue495Nested struct { + D int `json:"d"` + } + type issue495Value struct { + A int `json:"a"` + B string `json:"b"` + C *issue495Nested `json:"c"` + } + + const goroutineNum = 128 + var wg sync.WaitGroup + errCh := make(chan error, goroutineNum) + start := make(chan struct{}) + + wg.Add(goroutineNum) + for i := 0; i < goroutineNum; i++ { + go func() { + defer wg.Done() + defer func() { + if r := recover(); r != nil { + errCh <- fmt.Errorf("unexpected panic: %v", r) + } + }() + <-start + + var v issue495Value + if err := json.Unmarshal([]byte(`{"a":1,"b":"x","c":{"d":2}}`), &v); err != nil { + errCh <- err + return + } + if v.A != 1 || v.B != "x" || v.C == nil || v.C.D != 2 { + errCh <- fmt.Errorf("unexpected decoded value: %+v", v) + } + }() + } + + close(start) + wg.Wait() + close(errCh) + + for err := range errCh { + t.Error(err) + } +} diff --git a/internal/decoder/compile.go b/internal/decoder/compile.go index 8ad50936..b84aa2de 100644 --- a/internal/decoder/compile.go +++ b/internal/decoder/compile.go @@ -17,20 +17,40 @@ var ( jsonNumberType = reflect.TypeOf(json.Number("")) typeAddr *runtime.TypeAddr cachedDecoderMap unsafe.Pointer // map[uintptr]decoder - cachedDecoder []Decoder + cachedDecoder []atomic.Pointer[decoderCacheEntry] initOnce sync.Once ) +type decoderCacheEntry struct { + dec Decoder +} + func initDecoder() { initOnce.Do(func() { typeAddr = runtime.AnalyzeTypeAddr() if typeAddr == nil { typeAddr = &runtime.TypeAddr{} } - cachedDecoder = make([]Decoder, typeAddr.AddrRange>>typeAddr.AddrShift+1) + cachedDecoder = make([]atomic.Pointer[decoderCacheEntry], typeAddr.AddrRange>>typeAddr.AddrShift+1) }) } +func loadCachedDecoder(index uintptr) Decoder { + entry := cachedDecoder[index].Load() + if entry == nil { + return nil + } + return entry.dec +} + +func storeCachedDecoder(index uintptr, dec Decoder) Decoder { + entry := &decoderCacheEntry{dec: dec} + if cachedDecoder[index].CompareAndSwap(nil, entry) { + return dec + } + return loadCachedDecoder(index) +} + func loadDecoderMap() map[uintptr]Decoder { initDecoder() p := atomic.LoadPointer(&cachedDecoderMap) diff --git a/internal/decoder/compile_norace.go b/internal/decoder/compile_norace.go index 025ca85b..0f9e0203 100644 --- a/internal/decoder/compile_norace.go +++ b/internal/decoder/compile_norace.go @@ -17,7 +17,7 @@ func CompileToGetDecoder(typ *runtime.Type) (Decoder, error) { } index := (typeptr - typeAddr.BaseTypeAddr) >> typeAddr.AddrShift - if dec := cachedDecoder[index]; dec != nil { + if dec := loadCachedDecoder(index); dec != nil { return dec, nil } @@ -25,6 +25,5 @@ func CompileToGetDecoder(typ *runtime.Type) (Decoder, error) { if err != nil { return nil, err } - cachedDecoder[index] = dec - return dec, nil + return storeCachedDecoder(index, dec), nil } diff --git a/internal/decoder/compile_race.go b/internal/decoder/compile_race.go index 023b817c..39e01099 100644 --- a/internal/decoder/compile_race.go +++ b/internal/decoder/compile_race.go @@ -4,14 +4,11 @@ package decoder import ( - "sync" "unsafe" "github.com/goccy/go-json/internal/runtime" ) -var decMu sync.RWMutex - func CompileToGetDecoder(typ *runtime.Type) (Decoder, error) { initDecoder() typeptr := uintptr(unsafe.Pointer(typ)) @@ -20,19 +17,13 @@ func CompileToGetDecoder(typ *runtime.Type) (Decoder, error) { } index := (typeptr - typeAddr.BaseTypeAddr) >> typeAddr.AddrShift - decMu.RLock() - if dec := cachedDecoder[index]; dec != nil { - decMu.RUnlock() + if dec := loadCachedDecoder(index); dec != nil { return dec, nil } - decMu.RUnlock() dec, err := compileHead(typ, map[uintptr]Decoder{}) if err != nil { return nil, err } - decMu.Lock() - cachedDecoder[index] = dec - decMu.Unlock() - return dec, nil + return storeCachedDecoder(index, dec), nil } diff --git a/internal/decoder/compile_test.go b/internal/decoder/compile_test.go new file mode 100644 index 00000000..440d3803 --- /dev/null +++ b/internal/decoder/compile_test.go @@ -0,0 +1,111 @@ +package decoder + +import ( + "reflect" + "sync" + "testing" + "unsafe" + + "github.com/goccy/go-json/internal/runtime" +) + +type decoderCacheTestDecoder struct { + id int +} + +func (d *decoderCacheTestDecoder) Decode(*RuntimeContext, int64, int64, unsafe.Pointer) (int64, error) { + return 0, nil +} + +func (d *decoderCacheTestDecoder) DecodePath(*RuntimeContext, int64, int64) ([][]byte, int64, error) { + return nil, 0, nil +} + +func (d *decoderCacheTestDecoder) DecodeStream(*Stream, int64, unsafe.Pointer) error { + return nil +} + +type decoderCacheTestStruct struct { + A int `json:"a"` +} + +func TestDecoderFastCacheReturnsStoredDecoder(t *testing.T) { + index, ok := decoderCacheTestIndex() + if !ok { + t.Skip("decoder fast cache is disabled for this runtime") + } + clearDecoderCacheTestSlot(index) + defer clearDecoderCacheTestSlot(index) + + if got := loadCachedDecoder(index); got != nil { + t.Fatalf("loadCachedDecoder() = %T, want nil", got) + } + + first := &decoderCacheTestDecoder{id: 1} + if got := storeCachedDecoder(index, first); got != first { + t.Fatalf("storeCachedDecoder(first) = %p, want %p", got, first) + } + if got := loadCachedDecoder(index); got != first { + t.Fatalf("loadCachedDecoder() = %p, want %p", got, first) + } + + second := &decoderCacheTestDecoder{id: 2} + if got := storeCachedDecoder(index, second); got != first { + t.Fatalf("storeCachedDecoder(second) = %p, want cached %p", got, first) + } + if got := loadCachedDecoder(index); got != first { + t.Fatalf("loadCachedDecoder() after second store = %p, want %p", got, first) + } +} + +func TestDecoderFastCacheConcurrentStoresReturnWinner(t *testing.T) { + index, ok := decoderCacheTestIndex() + if !ok { + t.Skip("decoder fast cache is disabled for this runtime") + } + clearDecoderCacheTestSlot(index) + defer clearDecoderCacheTestSlot(index) + + const goroutineNum = 64 + start := make(chan struct{}) + results := make(chan Decoder, goroutineNum) + var wg sync.WaitGroup + wg.Add(goroutineNum) + + for i := 0; i < goroutineNum; i++ { + dec := &decoderCacheTestDecoder{id: i} + go func() { + defer wg.Done() + <-start + results <- storeCachedDecoder(index, dec) + }() + } + + close(start) + wg.Wait() + close(results) + + winner := loadCachedDecoder(index) + if winner == nil { + t.Fatal("loadCachedDecoder() = nil, want cached decoder") + } + for got := range results { + if got != winner { + t.Fatalf("storeCachedDecoder() = %p, want cached winner %p", got, winner) + } + } +} + +func decoderCacheTestIndex() (uintptr, bool) { + initDecoder() + typ := runtime.Type2RType(reflect.TypeOf((*decoderCacheTestStruct)(nil))) + typeptr := uintptr(unsafe.Pointer(typ)) + if typeptr > typeAddr.MaxTypeAddr || typeptr < typeAddr.BaseTypeAddr { + return 0, false + } + return (typeptr - typeAddr.BaseTypeAddr) >> typeAddr.AddrShift, true +} + +func clearDecoderCacheTestSlot(index uintptr) { + cachedDecoder[index].Store(nil) +}