Fix decoder cache publication race

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)
+	}
+}

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)

internal/decoder/compile_norace.go

@@ -17,14 +17,13 @@ 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
 	}
 
 	dec, err := compileHead(typ, map[uintptr]Decoder{})
 	if err != nil {
 		return nil, err
 	}
-	cachedDecoder[index] = dec
-	return dec, nil
+	return storeCachedDecoder(index, dec), nil
 }

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
 }

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)
+}