diff --git a/go_backend/extension_runtime_storage.go b/go_backend/extension_runtime_storage.go index 7c537355..b10363a2 100644 --- a/go_backend/extension_runtime_storage.go +++ b/go_backend/extension_runtime_storage.go @@ -95,6 +95,19 @@ func cloneJSONMap(source map[string]any) map[string]any { func readCachedJSONMapLocked( path string, load func() (map[string]any, error), +) (map[string]any, error) { + snapshot, err := cachedJSONMapLocked(path, load) + if err != nil { + return nil, err + } + return cloneJSONMap(snapshot), nil +} + +// cachedJSONMapLocked returns the shared, read-only cache entry. Never expose +// it to a VM or mutate it. Callers must hold the path-specific file mutex. +func cachedJSONMapLocked( + path string, + load func() (map[string]any, error), ) (map[string]any, error) { identity, err := extensionFileIdentityForPath(path) if err != nil { @@ -103,7 +116,7 @@ func readCachedJSONMapLocked( if cached, ok := extensionJSONCaches.Load(path); ok { entry := cached.(*extensionJSONCacheEntry) if entry.identity == identity { - return cloneJSONMap(entry.snapshot), nil + return entry.snapshot, nil } } @@ -113,11 +126,20 @@ func readCachedJSONMapLocked( } extensionJSONCaches.Store(path, &extensionJSONCacheEntry{ identity: identity, - snapshot: cloneJSONMap(snapshot), + snapshot: snapshot, }) return snapshot, nil } +func readCachedJSONValueLocked(path, key string, load func() (map[string]any, error)) (any, bool, error) { + snapshot, err := cachedJSONMapLocked(path, load) + if err != nil { + return nil, false, err + } + value, exists := snapshot[key] + return cloneJSONValue(value), exists, nil +} + func storeCachedJSONMapLocked(path string, snapshot map[string]any) error { identity, err := extensionFileIdentityForPath(path) if err != nil { @@ -223,14 +245,18 @@ func (r *extensionRuntime) storageGet(call goja.FunctionCall) goja.Value { key := call.Arguments[0].String() - if err := r.refreshStorage(); err != nil { + path := r.getStoragePath() + fileMu := extensionFileMu(path) + fileMu.Lock() + value, exists, err := readCachedJSONValueLocked(path, key, func() (map[string]any, error) { + return readJSONMapFile(path) + }) + fileMu.Unlock() + if err != nil { GoLog("[Extension:%s] Storage load error: %v\n", r.extensionID, err) return goja.Undefined() } - r.storageMu.RLock() - value, exists := r.storageCache[key] - r.storageMu.RUnlock() if !exists { if len(call.Arguments) > 1 { return call.Arguments[1] diff --git a/go_backend/extension_runtime_storage_test.go b/go_backend/extension_runtime_storage_test.go index 9a6890c7..fa0a4f1b 100644 --- a/go_backend/extension_runtime_storage_test.go +++ b/go_backend/extension_runtime_storage_test.go @@ -3,6 +3,7 @@ package gobackend import ( "bytes" "encoding/json" + "fmt" "os" "path/filepath" "testing" @@ -68,6 +69,79 @@ func TestExtensionJSONCacheUsesIdentityAndIsolatesSnapshots(t *testing.T) { } } +func TestExtensionStorageKeyReadIsolatedAndFresh(t *testing.T) { + dataDir := t.TempDir() + ext := &loadedExtension{ID: "key-read", Manifest: &ExtensionManifest{Name: "key-read"}, DataDir: dataDir} + a := newExtensionRuntime(ext) + b := newExtensionRuntime(ext) + a.RegisterAPIs(goja.New()) + b.RegisterAPIs(goja.New()) + setStorageValue(t, a, "nested", map[string]any{"items": []any{map[string]any{"value": "original"}}}) + read := func(r *extensionRuntime, key string) goja.Value { + return r.storageGet(goja.FunctionCall{Arguments: []goja.Value{r.vm.ToValue(key)}}) + } + value := read(a, "nested").ToObject(a.vm) + items := value.Get("items").ToObject(a.vm) + if err := items.Get("0").ToObject(a.vm).Set("value", "local mutation"); err != nil { + t.Fatal(err) + } + for _, runtime := range []*extensionRuntime{a, b} { + got := read(runtime, "nested").ToObject(runtime.vm).Get("items").ToObject(runtime.vm).Get("0").ToObject(runtime.vm).Get("value").String() + if got != "original" { + t.Fatalf("VM mutation leaked: %q", got) + } + } + setStorageValue(t, b, "nested", "updated") + if got := read(a, "nested").String(); got != "updated" { + t.Fatalf("cross-runtime update not visible: %q", got) + } + if err := os.WriteFile(filepath.Join(dataDir, "storage.json"), []byte(`{"nested":"external replacement","nullValue":null}`), 0600); err != nil { + t.Fatal(err) + } + if got := read(a, "nested").String(); got != "external replacement" { + t.Fatalf("external replacement not visible: %q", got) + } + if !goja.IsNull(read(a, "nullValue")) || !goja.IsUndefined(read(a, "absent")) { + t.Fatal("null and absent values must remain distinct") + } + if err := os.Remove(filepath.Join(dataDir, "storage.json")); err != nil { + t.Fatal(err) + } + if !goja.IsUndefined(read(a, "nested")) { + t.Fatal("removed storage file reused stale value") + } +} + +func BenchmarkExtensionCachedSingleKeyRead(b *testing.B) { + for _, size := range []int{10, 10000} { + b.Run(fmt.Sprintf("entries_%d", size), func(b *testing.B) { + path := filepath.Join(b.TempDir(), "storage.json") + snapshot := map[string]any{"token": "value"} + for i := 0; i < size; i++ { + snapshot[fmt.Sprint(i)] = map[string]any{"items": []any{"large cached value", i}} + } + data, _ := json.Marshal(snapshot) + if err := os.WriteFile(path, data, 0600); err != nil { + b.Fatal(err) + } + load := func() (map[string]any, error) { return readJSONMapFile(path) } + mu := extensionFileMu(path) + mu.Lock() + defer mu.Unlock() + if _, _, err := readCachedJSONValueLocked(path, "token", load); err != nil { + b.Fatal(err) + } + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + if _, _, err := readCachedJSONValueLocked(path, "token", load); err != nil { + b.Fatal(err) + } + } + }) + } +} + func TestExtensionRuntimeStorageConcurrentRuntimesMergeWrites(t *testing.T) { dataDir := t.TempDir() ext := &loadedExtension{ID: "merge-test", Manifest: &ExtensionManifest{Name: "merge-test"}, DataDir: dataDir}