From 5443e876cb2bde545557466a0906be9a6556ab3f Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Thu, 6 Jan 2022 16:43:07 -0800 Subject: [PATCH 1/3] fix an data race cause by early unlock --- api/service/legacysync/syncing.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/api/service/legacysync/syncing.go b/api/service/legacysync/syncing.go index 447f29999f..5a9b9a0a82 100644 --- a/api/service/legacysync/syncing.go +++ b/api/service/legacysync/syncing.go @@ -1195,8 +1195,9 @@ func getSyncStatusExpiration(role nodeconfig.Role) time.Duration { func (status *syncStatus) Get(fallback func() SyncCheckResult) SyncCheckResult { status.lock.RLock() if !status.expired() { + result := status.lastResult status.lock.RUnlock() - return status.lastResult + return result } status.lock.RUnlock() From a433d277f948f87e6823537001bfd73eac748d90 Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Thu, 6 Jan 2022 17:46:02 -0800 Subject: [PATCH 2/3] add a concurrency test for syncing cache --- api/service/legacysync/syncing_test.go | 45 ++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/api/service/legacysync/syncing_test.go b/api/service/legacysync/syncing_test.go index 3a17c82bad..1d1e19a292 100644 --- a/api/service/legacysync/syncing_test.go +++ b/api/service/legacysync/syncing_test.go @@ -6,7 +6,10 @@ import ( "math/big" "reflect" "strings" + "sync" + "sync/atomic" "testing" + "time" nodeconfig "github.com/harmony-one/harmony/internal/configs/node" @@ -250,3 +253,45 @@ func makeTestBlock(bn uint64, parentHash common.Hash) *types.Block { block := types.NewBlockWithHeader(testHeader) return block } + +func TestSyncStatus_Get_Concurrency(t *testing.T) { + ss := newSyncStatus(nodeconfig.Validator) + ss.expiration = 2 * time.Second + var ( + total int32 + updated int32 + wg sync.WaitGroup + stop = make(chan struct{}) + ) + + fb := func() SyncCheckResult { + time.Sleep(1 * time.Second) + atomic.AddInt32(&updated, 1) + return SyncCheckResult{IsInSync: true} + } + for i := 0; i != 20; i++ { + wg.Add(1) + go func() { + defer wg.Done() + + t := time.NewTicker(20 * time.Millisecond) + defer t.Stop() + for { + select { + case <-stop: + return + case <-t.C: + atomic.AddInt32(&total, 1) + ss.Get(fb) + } + } + }() + } + + time.Sleep(10 * time.Second) + close(stop) + wg.Wait() + + fmt.Printf("updated %v times\n", updated) + fmt.Printf("total %v times\n", total) +} From 1a505a17d7b334bbfbe187b8c3a7bdbd728aa66c Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Thu, 6 Jan 2022 17:57:31 -0800 Subject: [PATCH 3/3] skip the concurrent test for sync status --- api/service/legacysync/syncing_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/api/service/legacysync/syncing_test.go b/api/service/legacysync/syncing_test.go index 1d1e19a292..63f2f5705c 100644 --- a/api/service/legacysync/syncing_test.go +++ b/api/service/legacysync/syncing_test.go @@ -255,6 +255,8 @@ func makeTestBlock(bn uint64, parentHash common.Hash) *types.Block { } func TestSyncStatus_Get_Concurrency(t *testing.T) { + t.Skip() + ss := newSyncStatus(nodeconfig.Validator) ss.expiration = 2 * time.Second var (