Преглед изворни кода

fix(locale): protect translation catalog from concurrent map writes

getTranslationDict lazily populates the package-level defaultCatalog
map and runs on concurrent request goroutines via template functions,
flash messages, and error translation. Two concurrent requests for
languages not yet cached triggered Go's fatal "concurrent map writes"
and killed the daemon.

Guard the catalog with a sync.RWMutex using double-checked locking so
each language is loaded once. A failed load is no longer cached as an
empty dictionary, so unknown languages now return the error on every
call; both callers treat an error and an empty dictionary identically,
so rendered output is unchanged.

Add a regression test exercising concurrent lazy population; it fails
under the race detector on the previous implementation.
Fred пре 2 месеци
родитељ
комит
0e26f12426
2 измењених фајлова са 56 додато и 7 уклоњено
  1. 26 7
      internal/locale/catalog.go
  2. 30 0
      internal/locale/catalog_test.go

+ 26 - 7
internal/locale/catalog.go

@@ -7,6 +7,7 @@ import (
 	"embed"
 	"encoding/json"
 	"fmt"
+	"sync"
 )
 
 type translationDict struct {
@@ -15,19 +16,37 @@ type translationDict struct {
 }
 type catalog map[string]translationDict
 
-var defaultCatalog = make(catalog, len(AvailableLanguages))
+// defaultCatalog is populated lazily by getTranslationDict, which runs on
+// concurrent request goroutines, so every access must hold defaultCatalogMutex.
+var (
+	defaultCatalog      = make(catalog, len(AvailableLanguages))
+	defaultCatalogMutex sync.RWMutex
+)
 
 //go:embed translations/*.json
 var translationFiles embed.FS
 
 func getTranslationDict(language string) (translationDict, error) {
-	if _, ok := defaultCatalog[language]; !ok {
-		var err error
-		if defaultCatalog[language], err = loadTranslationFile(language); err != nil {
-			return translationDict{}, err
-		}
+	defaultCatalogMutex.RLock()
+	dict, found := defaultCatalog[language]
+	defaultCatalogMutex.RUnlock()
+	if found {
+		return dict, nil
+	}
+
+	defaultCatalogMutex.Lock()
+	defer defaultCatalogMutex.Unlock()
+
+	if dict, found := defaultCatalog[language]; found {
+		return dict, nil
+	}
+
+	dict, err := loadTranslationFile(language)
+	if err != nil {
+		return translationDict{}, err
 	}
-	return defaultCatalog[language], nil
+	defaultCatalog[language] = dict
+	return dict, nil
 }
 
 func loadTranslationFile(language string) (translationDict, error) {

+ 30 - 0
internal/locale/catalog_test.go

@@ -4,6 +4,7 @@
 package locale // import "miniflux.app/v2/internal/locale"
 
 import (
+	"sync"
 	"testing"
 )
 
@@ -30,6 +31,35 @@ func TestParser(t *testing.T) {
 	}
 }
 
+// TestGetTranslationDictConcurrency exercises the lazy population of the
+// catalog from concurrent goroutines, as HTTP request handlers do. It must be
+// run with the race detector enabled to catch unsynchronized catalog access.
+func TestGetTranslationDictConcurrency(t *testing.T) {
+	defaultCatalog = make(catalog, len(AvailableLanguages))
+
+	const iterations = 10
+
+	var wg sync.WaitGroup
+	for i := 0; i < iterations; i++ {
+		for language := range AvailableLanguages {
+			wg.Add(1)
+			go func(language string) {
+				defer wg.Done()
+
+				dict, err := getTranslationDict(language)
+				if err != nil {
+					t.Errorf(`Unable to get translation dictionary for language %q: %v`, language, err)
+					return
+				}
+				if len(dict.singulars) == 0 {
+					t.Errorf(`The translation dictionary for language %q should not be empty`, language)
+				}
+			}(language)
+		}
+	}
+	wg.Wait()
+}
+
 func TestLoadCatalog(t *testing.T) {
 	for language := range AvailableLanguages {
 		_, err := loadTranslationFile(language)