From 153f674b19acf2949a60d5c9ae21698081478c39 Mon Sep 17 00:00:00 2001 From: liubo02 Date: Fri, 25 Apr 2025 17:55:25 +0800 Subject: [PATCH 1/3] fix(race): fix race Signed-off-by: liubo02 --- Makefile | 4 ++-- pkg/timanager/manager_test.go | 2 +- pkg/timanager/poller_test.go | 24 +++++++++++++++++++----- pkg/timanager/util.go | 9 +-------- 4 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Makefile b/Makefile index d365cde6990..d50ffda1af7 100644 --- a/Makefile +++ b/Makefile @@ -129,8 +129,8 @@ lint-fix: bin/golangci-lint .PHONY: unit unit: - cd $(VALIDATION_TEST_PATH) && go test ./... - go test $$(go list -e ./... | grep -v cmd | grep -v tools | grep -v tests/e2e | grep -v third_party) \ + cd $(VALIDATION_TEST_PATH) && go test -race ./... + go test -race $$(go list -e ./... | grep -v cmd | grep -v tools | grep -v tests/e2e | grep -v third_party) \ -cover -coverprofile=coverage.txt -covermode=atomic sed -i.bak '/generated/d;/fake.go/d' coverage.txt && rm coverage.txt.bak diff --git a/pkg/timanager/manager_test.go b/pkg/timanager/manager_test.go index 1997d1c4fa3..7bd2a444af4 100644 --- a/pkg/timanager/manager_test.go +++ b/pkg/timanager/manager_test.go @@ -325,7 +325,7 @@ func TestClientManagerSource(t *testing.T) { }) assert.True(tt, synced) - lister.L.Items = c.updated + lister.UpdateItems(c.updated) select { case <-ctx.Done(): diff --git a/pkg/timanager/poller_test.go b/pkg/timanager/poller_test.go index 6a899e61486..d7cf6f6e7de 100644 --- a/pkg/timanager/poller_test.go +++ b/pkg/timanager/poller_test.go @@ -18,6 +18,7 @@ import ( "cmp" "context" "slices" + "sync" "testing" "time" @@ -33,17 +34,23 @@ import ( ) type FakeLister[T any, PT Object[T]] struct { - L List[T, PT] + lock sync.Mutex + L List[T, PT] } func (l *FakeLister[T, PT]) List(_ context.Context) (*List[T, PT], error) { + l.lock.Lock() + defer l.lock.Unlock() return &l.L, nil } -func (l *FakeLister[T, PT]) GetItems(_ *List[T, PT]) []PT { - objs := make([]PT, 0, len(l.L.Items)) - for i := range l.L.Items { - objs = append(objs, &l.L.Items[i]) +func (l *FakeLister[T, PT]) GetItems(list *List[T, PT]) []PT { + l.lock.Lock() + defer l.lock.Unlock() + + objs := make([]PT, 0, len(list.Items)) + for i := range list.Items { + objs = append(objs, &list.Items[i]) } return objs } @@ -52,6 +59,13 @@ func (*FakeLister[T, PT]) MarkAsInvalid(PT) bool { return false } +func (l *FakeLister[T, PT]) UpdateItems(items []T) { + l.lock.Lock() + defer l.lock.Unlock() + + l.L.Items = items +} + func TestPoller(t *testing.T) { cases := []struct { desc string diff --git a/pkg/timanager/util.go b/pkg/timanager/util.go index 574d8137a20..8c936759108 100644 --- a/pkg/timanager/util.go +++ b/pkg/timanager/util.go @@ -131,8 +131,6 @@ type cached[Client, UnderlayClient any] struct { c Client f SharedInformerFactory[UnderlayClient] - cancel context.CancelFunc - cacheKeys []string } @@ -157,14 +155,9 @@ func (c *cached[Client, UnderlayClient]) Keys() []string { } func (c *cached[Client, UnderlayClient]) Start(ctx context.Context) { - nctx, cancel := context.WithCancel(ctx) - c.cancel = cancel - c.f.Start(nctx.Done()) + c.f.Start(ctx.Done()) } func (c *cached[Client, UnderlayClient]) Stop() { - if c.cancel != nil { - c.cancel() - } c.f.Shutdown() } From 4c7c4a1a959831104aa41728418f6470880cb447 Mon Sep 17 00:00:00 2001 From: liubo02 Date: Sun, 27 Apr 2025 16:56:07 +0800 Subject: [PATCH 2/3] fix cancel Signed-off-by: liubo02 --- pkg/timanager/util.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/timanager/util.go b/pkg/timanager/util.go index 8c936759108..620b59db2f2 100644 --- a/pkg/timanager/util.go +++ b/pkg/timanager/util.go @@ -132,6 +132,8 @@ type cached[Client, UnderlayClient any] struct { f SharedInformerFactory[UnderlayClient] cacheKeys []string + + stopCh chan struct{} } func NewCache[Client, UnderlayClient any](keys []string, c Client, f SharedInformerFactory[UnderlayClient]) Cache[Client, UnderlayClient] { @@ -139,6 +141,7 @@ func NewCache[Client, UnderlayClient any](keys []string, c Client, f SharedInfor c: c, f: f, cacheKeys: keys, + stopCh: make(chan struct{}), } } @@ -155,9 +158,15 @@ func (c *cached[Client, UnderlayClient]) Keys() []string { } func (c *cached[Client, UnderlayClient]) Start(ctx context.Context) { - c.f.Start(ctx.Done()) + go func() { + <-ctx.Done() + c.Stop() + }() + + c.f.Start(c.stopCh) } func (c *cached[Client, UnderlayClient]) Stop() { + close(c.stopCh) c.f.Shutdown() } From 297d6101e60ca6a0475b4be4cbb2d40c2a2bd32e Mon Sep 17 00:00:00 2001 From: liubo02 Date: Sun, 27 Apr 2025 18:00:12 +0800 Subject: [PATCH 3/3] fix cancel Signed-off-by: liubo02 --- pkg/timanager/util.go | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/pkg/timanager/util.go b/pkg/timanager/util.go index 620b59db2f2..66308001256 100644 --- a/pkg/timanager/util.go +++ b/pkg/timanager/util.go @@ -133,7 +133,9 @@ type cached[Client, UnderlayClient any] struct { cacheKeys []string - stopCh chan struct{} + started bool + cancel context.CancelFunc + lock sync.Mutex } func NewCache[Client, UnderlayClient any](keys []string, c Client, f SharedInformerFactory[UnderlayClient]) Cache[Client, UnderlayClient] { @@ -141,7 +143,6 @@ func NewCache[Client, UnderlayClient any](keys []string, c Client, f SharedInfor c: c, f: f, cacheKeys: keys, - stopCh: make(chan struct{}), } } @@ -158,15 +159,28 @@ func (c *cached[Client, UnderlayClient]) Keys() []string { } func (c *cached[Client, UnderlayClient]) Start(ctx context.Context) { - go func() { - <-ctx.Done() - c.Stop() - }() + c.lock.Lock() + defer c.lock.Unlock() - c.f.Start(c.stopCh) + if c.started { + return + } + + nctx, cancel := context.WithCancel(ctx) + c.cancel = cancel + c.started = true + + c.f.Start(nctx.Done()) } func (c *cached[Client, UnderlayClient]) Stop() { - close(c.stopCh) + c.lock.Lock() + defer c.lock.Unlock() + + if !c.started { + return + } + + c.cancel() c.f.Shutdown() }