mirror of
https://github.com/NoFxAiOS/nofx.git
synced 2026-07-15 00:36:56 +08:00
fix(trader): add mutex to prevent race condition in Meta refresh (#796)
* fix(trader): add mutex to prevent race condition in Meta refresh (issue #742) **問題**: 根據 issue #742 審查標準,發現 BLOCKING 級別的並發安全問題: - refreshMetaIfNeeded() 中的 `t.meta = meta` 缺少並發保護 - 多個 goroutine 同時調用 OpenLong/OpenShort 會造成競態條件 **修復**: 1. 添加 sync.RWMutex 保護 meta 字段 2. refreshMetaIfNeeded() 使用寫鎖保護 meta 更新 3. getSzDecimals() 使用讀鎖保護 meta 訪問 **符合標準**: - issue #742: "並發安全問題需使用 sync.Once 等機制" - 使用 RWMutex 實現讀寫分離,提升並發性能 Co-Authored-By: tinkle-community <tinklefund@gmail.com> * test(trader): add comprehensive race condition tests for meta field mutex protection - Test concurrent reads (100 goroutines accessing getSzDecimals) - Test concurrent read/write (50 readers + 10 writers simulating meta refresh) - Test nil meta edge case (returns default value 4) - Test valid meta with multiple coins (BTC, ETH, SOL) - Test massive concurrency (1000 iterations with race detector) All 5 test cases passed, including -race verification with no data races detected. Co-Authored-By: tinkle-community <tinklefund@gmail.com> --------- Co-authored-by: ZhouYongyou <128128010+zhouyongyou@users.noreply.github.com> Co-authored-by: tinkle-community <tinklefund@gmail.com>
This commit is contained in:
committed by
GitHub
parent
6a66913194
commit
57e31b2ace
192
trader/hyperliquid_trader_race_test.go
Normal file
192
trader/hyperliquid_trader_race_test.go
Normal file
@@ -0,0 +1,192 @@
|
||||
package trader
|
||||
|
||||
import (
|
||||
"context"
|
||||
"sync"
|
||||
"testing"
|
||||
|
||||
"github.com/sonirico/go-hyperliquid"
|
||||
)
|
||||
|
||||
// TestMetaConcurrentAccess tests that concurrent access to meta field is safe
|
||||
func TestMetaConcurrentAccess(t *testing.T) {
|
||||
// Create a HyperliquidTrader instance with meta initialized
|
||||
trader := &HyperliquidTrader{
|
||||
ctx: context.Background(),
|
||||
meta: &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5},
|
||||
{Name: "ETH", SzDecimals: 4},
|
||||
},
|
||||
},
|
||||
metaMutex: sync.RWMutex{},
|
||||
}
|
||||
|
||||
// Number of concurrent goroutines
|
||||
concurrency := 100
|
||||
var wg sync.WaitGroup
|
||||
|
||||
// Test concurrent reads (getSzDecimals)
|
||||
for i := 0; i < concurrency; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
// This should not cause race conditions
|
||||
decimals := trader.getSzDecimals("BTC")
|
||||
if decimals != 5 {
|
||||
t.Errorf("Expected decimals 5, got %d", decimals)
|
||||
}
|
||||
}()
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
}
|
||||
|
||||
// TestMetaConcurrentReadWrite tests concurrent reads and writes to meta field
|
||||
func TestMetaConcurrentReadWrite(t *testing.T) {
|
||||
trader := &HyperliquidTrader{
|
||||
ctx: context.Background(),
|
||||
meta: &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5},
|
||||
},
|
||||
},
|
||||
metaMutex: sync.RWMutex{},
|
||||
}
|
||||
|
||||
var wg sync.WaitGroup
|
||||
concurrency := 50
|
||||
|
||||
// Concurrent readers
|
||||
for i := 0; i < concurrency; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
trader.getSzDecimals("BTC")
|
||||
}()
|
||||
}
|
||||
|
||||
// Concurrent writers (simulating meta refresh)
|
||||
for i := 0; i < 10; i++ {
|
||||
wg.Add(1)
|
||||
go func(iteration int) {
|
||||
defer wg.Done()
|
||||
// Simulate meta update
|
||||
trader.metaMutex.Lock()
|
||||
trader.meta = &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5 + iteration%3},
|
||||
{Name: "ETH", SzDecimals: 4},
|
||||
},
|
||||
}
|
||||
trader.metaMutex.Unlock()
|
||||
}(i)
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
|
||||
// Verify meta is not nil after all operations
|
||||
trader.metaMutex.RLock()
|
||||
if trader.meta == nil {
|
||||
t.Error("Meta should not be nil after concurrent operations")
|
||||
}
|
||||
trader.metaMutex.RUnlock()
|
||||
}
|
||||
|
||||
// TestGetSzDecimals_NilMeta tests getSzDecimals with nil meta
|
||||
func TestGetSzDecimals_NilMeta(t *testing.T) {
|
||||
trader := &HyperliquidTrader{
|
||||
meta: nil,
|
||||
metaMutex: sync.RWMutex{},
|
||||
}
|
||||
|
||||
// Should return default value 4 when meta is nil
|
||||
decimals := trader.getSzDecimals("BTC")
|
||||
expectedDecimals := 4
|
||||
|
||||
if decimals != expectedDecimals {
|
||||
t.Errorf("Expected default decimals %d for nil meta, got %d", expectedDecimals, decimals)
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetSzDecimals_ValidMeta tests getSzDecimals with valid meta
|
||||
func TestGetSzDecimals_ValidMeta(t *testing.T) {
|
||||
trader := &HyperliquidTrader{
|
||||
meta: &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5},
|
||||
{Name: "ETH", SzDecimals: 4},
|
||||
{Name: "SOL", SzDecimals: 3},
|
||||
},
|
||||
},
|
||||
metaMutex: sync.RWMutex{},
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
coin string
|
||||
expectedDecimals int
|
||||
}{
|
||||
{"BTC", 5},
|
||||
{"ETH", 4},
|
||||
{"SOL", 3},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.coin, func(t *testing.T) {
|
||||
decimals := trader.getSzDecimals(tt.coin)
|
||||
if decimals != tt.expectedDecimals {
|
||||
t.Errorf("For coin %s, expected decimals %d, got %d", tt.coin, tt.expectedDecimals, decimals)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestMetaMutex_NoRaceCondition tests that using -race detector finds no issues
|
||||
// Run with: go test -race -run TestMetaMutex_NoRaceCondition
|
||||
func TestMetaMutex_NoRaceCondition(t *testing.T) {
|
||||
trader := &HyperliquidTrader{
|
||||
ctx: context.Background(),
|
||||
meta: &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5},
|
||||
{Name: "ETH", SzDecimals: 4},
|
||||
},
|
||||
},
|
||||
metaMutex: sync.RWMutex{},
|
||||
}
|
||||
|
||||
var wg sync.WaitGroup
|
||||
iterations := 1000
|
||||
|
||||
// Massive concurrent reads
|
||||
for i := 0; i < iterations; i++ {
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
trader.getSzDecimals("BTC")
|
||||
trader.getSzDecimals("ETH")
|
||||
}()
|
||||
}
|
||||
|
||||
// Concurrent writes
|
||||
for i := 0; i < 100; i++ {
|
||||
wg.Add(1)
|
||||
go func(idx int) {
|
||||
defer wg.Done()
|
||||
trader.metaMutex.Lock()
|
||||
trader.meta = &hyperliquid.Meta{
|
||||
Universe: []hyperliquid.AssetInfo{
|
||||
{Name: "BTC", SzDecimals: 5},
|
||||
{Name: "ETH", SzDecimals: 4},
|
||||
{Name: "SOL", SzDecimals: 3},
|
||||
},
|
||||
}
|
||||
trader.metaMutex.Unlock()
|
||||
}(i)
|
||||
}
|
||||
|
||||
wg.Wait()
|
||||
|
||||
// If we reach here without race detector errors, the test passes
|
||||
t.Log("No race conditions detected in concurrent meta access")
|
||||
}
|
||||
Reference in New Issue
Block a user