mirror of
https://github.com/chenhg5/cc-connect.git
synced 2026-10-25 17:36:43 +08:00
fix(core/i18n): protect lang/detected fields with mutex to fix data race (#922)
I18n had no synchronization on its mutable state (lang, detected,
saveFunc) but the type is shared across every concurrent platform
message handler:
- DetectAndSet writes i.detected — called from
processInteractiveMessageWith and the queued-message dequeue paths
in core/engine.go (3 call sites). Two messages arriving
simultaneously from different platforms means two unsynchronized
writes to the same string field.
- T / CurrentLang / IsZhLike read i.lang and i.detected — called
from dozens of code paths (replies, footers, error messages, mode
switches), so concurrent reads happen alongside the writes
DetectAndSet performs.
- SetLang writes both fields — driven by the management API.
`go test -race` against a 100-iteration goroutine pair calling
DetectAndSet flagged the race directly on i.detected:
WARNING: DATA RACE
Read at 0x...: core.(*I18n).DetectAndSet
Previous write at 0x...: core.(*I18n).DetectAndSet
Add sync.RWMutex to I18n. DetectAndSet uses an RLock-fast-path so the
common case (lang already detected, no change) is read-only; only the
mutating path takes the write lock, and re-checks under it to avoid
losing a concurrent update. T / CurrentLang / IsZhLike take RLock for
the duration of their read; SetLang and SetSaveFunc take the write
lock. currentLang stays unexported and assumes the caller holds a
lock — every public caller now does.
Add TestI18n_ConcurrentAccess: 16 goroutines × 200 iterations
fanning DetectAndSet / T / CurrentLang / IsZhLike across four input
languages. Confirmed the test fails on main with the race detector
("race detected during execution of test") and passes on this branch.
This commit is contained in:
1 parent
973b0cca94
commit
f34d27ad7b
2 files changed
+94
-10
No files matched your search
+51
-9
@@ -1,6 +1,9 @@
|
||||
package core
|
||||
|
||||
import "fmt"
|
||||
import (
|
||||
"fmt"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// Language represents a supported language
|
||||
type Language string
|
||||
@@ -14,8 +17,15 @@ const (
|
||||
LangSpanish Language = "es"
|
||||
)
|
||||
|
||||
// I18n provides internationalized messages
|
||||
// I18n provides internationalized messages.
|
||||
//
|
||||
// All exported methods are safe to call from multiple goroutines: cc-connect
|
||||
// fans out platform message handlers concurrently, all of which can call
|
||||
// DetectAndSet (writes `detected`) and T / CurrentLang (read `lang`/`detected`)
|
||||
// at the same time. Without the mutex `go test -race` flags real data races
|
||||
// on the language fields.
|
||||
type I18n struct {
|
||||
mu sync.RWMutex
|
||||
lang Language
|
||||
detected Language
|
||||
saveFunc func(Language) error
|
||||
@@ -26,6 +36,8 @@ func NewI18n(lang Language) *I18n {
|
||||
}
|
||||
|
||||
func (i *I18n) SetSaveFunc(fn func(Language) error) {
|
||||
i.mu.Lock()
|
||||
defer i.mu.Unlock()
|
||||
i.saveFunc = fn
|
||||
}
|
||||
|
||||
@@ -76,21 +88,41 @@ func isSpanishHint(text string) bool {
|
||||
}
|
||||
|
||||
func (i *I18n) DetectAndSet(text string) {
|
||||
i.mu.RLock()
|
||||
if i.lang != LangAuto {
|
||||
i.mu.RUnlock()
|
||||
return
|
||||
}
|
||||
currentDetected := i.detected
|
||||
i.mu.RUnlock()
|
||||
|
||||
detected := DetectLanguage(text)
|
||||
if i.detected != detected {
|
||||
i.detected = detected
|
||||
if i.saveFunc != nil {
|
||||
if err := i.saveFunc(detected); err != nil {
|
||||
fmt.Printf("failed to save language: %v\n", err)
|
||||
}
|
||||
if currentDetected == detected {
|
||||
return
|
||||
}
|
||||
|
||||
i.mu.Lock()
|
||||
// Re-check under the write lock — another goroutine may have updated
|
||||
// i.detected between our RUnlock above and the Lock here.
|
||||
if i.lang != LangAuto || i.detected == detected {
|
||||
i.mu.Unlock()
|
||||
return
|
||||
}
|
||||
i.detected = detected
|
||||
saveFunc := i.saveFunc
|
||||
i.mu.Unlock()
|
||||
|
||||
if saveFunc != nil {
|
||||
if err := saveFunc(detected); err != nil {
|
||||
fmt.Printf("failed to save language: %v\n", err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (i *I18n) currentLang() Language {
|
||||
// Caller holds either no lock (most public methods take RLock and call
|
||||
// this) or RLock; this helper just reads the protected fields. All
|
||||
// public methods that call currentLang() acquire i.mu.RLock first.
|
||||
if i.lang == LangAuto {
|
||||
if i.detected != "" {
|
||||
return i.detected
|
||||
@@ -101,16 +133,24 @@ func (i *I18n) currentLang() Language {
|
||||
}
|
||||
|
||||
// CurrentLang returns the resolved language (exported for mode display).
|
||||
func (i *I18n) CurrentLang() Language { return i.currentLang() }
|
||||
func (i *I18n) CurrentLang() Language {
|
||||
i.mu.RLock()
|
||||
defer i.mu.RUnlock()
|
||||
return i.currentLang()
|
||||
}
|
||||
|
||||
// IsZhLike returns true for Simplified and Traditional Chinese.
|
||||
func (i *I18n) IsZhLike() bool {
|
||||
i.mu.RLock()
|
||||
defer i.mu.RUnlock()
|
||||
l := i.currentLang()
|
||||
return l == LangChinese || l == LangTraditionalChinese
|
||||
}
|
||||
|
||||
// SetLang overrides the language (disabling auto-detect).
|
||||
func (i *I18n) SetLang(lang Language) {
|
||||
i.mu.Lock()
|
||||
defer i.mu.Unlock()
|
||||
i.lang = lang
|
||||
i.detected = ""
|
||||
}
|
||||
@@ -3906,7 +3946,9 @@ var messages = map[MsgKey]map[Language]string{
|
||||
}
|
||||
|
||||
func (i *I18n) T(key MsgKey) string {
|
||||
i.mu.RLock()
|
||||
lang := i.currentLang()
|
||||
i.mu.RUnlock()
|
||||
if msg, ok := messages[key]; ok {
|
||||
if translated, ok := msg[lang]; ok {
|
||||
return translated
|
||||
|
||||
+43
-1
@@ -1,6 +1,9 @@
|
||||
package core
|
||||
|
||||
import "testing"
|
||||
import (
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestI18n_DefaultLanguage(t *testing.T) {
|
||||
i := NewI18n(LangEnglish)
|
||||
@@ -104,6 +107,45 @@ func TestIsChinese(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestI18n_ConcurrentAccess is a regression test for a real data race on
|
||||
// I18n's lang/detected fields. cc-connect fans out platform message
|
||||
// handlers concurrently; each one calls DetectAndSet (writes detected),
|
||||
// while typing/reply paths call T / CurrentLang concurrently (read
|
||||
// lang/detected). Without a mutex `go test -race` flagged real races on
|
||||
// these fields. This test exercises the same access pattern under -race
|
||||
// so a future edit that drops the mutex breaks loudly instead of
|
||||
// silently re-introducing the race.
|
||||
func TestI18n_ConcurrentAccess(t *testing.T) {
|
||||
i := NewI18n(LangAuto)
|
||||
i.SetSaveFunc(func(Language) error { return nil })
|
||||
|
||||
const goroutines = 16
|
||||
const iterations = 200
|
||||
|
||||
texts := []string{"hello world", "你好世界", "こんにちは", "¡Hola amigo!"}
|
||||
|
||||
var wg sync.WaitGroup
|
||||
for g := 0; g < goroutines; g++ {
|
||||
wg.Add(1)
|
||||
go func(g int) {
|
||||
defer wg.Done()
|
||||
for n := 0; n < iterations; n++ {
|
||||
switch (g + n) % 4 {
|
||||
case 0:
|
||||
i.DetectAndSet(texts[n%len(texts)])
|
||||
case 1:
|
||||
_ = i.T(MsgStarting)
|
||||
case 2:
|
||||
_ = i.CurrentLang()
|
||||
case 3:
|
||||
_ = i.IsZhLike()
|
||||
}
|
||||
}
|
||||
}(g)
|
||||
}
|
||||
wg.Wait()
|
||||
}
|
||||
|
||||
func TestIsJapanese(t *testing.T) {
|
||||
// Hiragana
|
||||
if !isJapanese('あ') {
|
||||
|
||||
Reference in new issue
Block a user