From b22fda4636eec84fe89d9739096bfc8269841c95 Mon Sep 17 00:00:00 2001 From: Max Smythe <smythe@google.com> Date: Wed, 23 Aug 2023 19:37:01 -0700 Subject: [PATCH] fix: Remove readiness tracker deadlock caused by duplicate syncs (#2970) Signed-off-by: Max Smythe <smythe@google.com> --- pkg/readiness/object_tracker.go | 5 +++++ pkg/readiness/object_tracker_test.go | 22 ++++++++++++++++++++++ pkg/readiness/tracker_map.go | 4 ++++ 3 files changed, 31 insertions(+) diff --git a/pkg/readiness/object_tracker.go b/pkg/readiness/object_tracker.go index 994413a3455..674824106a5 100644 --- a/pkg/readiness/object_tracker.go +++ b/pkg/readiness/object_tracker.go @@ -108,6 +108,11 @@ func (t *objectTracker) Expect(o runtime.Object) { return } + // Satisfied objects cannot be expected again. + if _, ok := t.satisfied[k]; ok { + return + } + // We may have seen it before starting to expect it if _, ok := t.seen[k]; ok { delete(t.seen, k) diff --git a/pkg/readiness/object_tracker_test.go b/pkg/readiness/object_tracker_test.go index 951f3151307..e6e601cd417 100644 --- a/pkg/readiness/object_tracker_test.go +++ b/pkg/readiness/object_tracker_test.go @@ -178,6 +178,28 @@ func Test_ObjectTracker_Duplicate_Expectations(t *testing.T) { } } +// Verify that that satisfied expectations cannot be re-established. +func Test_ObjectTracker_Satisfaction_Final(t *testing.T) { + ot := newObjTracker(schema.GroupVersionKind{}, nil) + + const count = 10 + ct := makeCTSlice("ct-", count) + for i := 0; i < len(ct); i++ { + ot.Expect(ct[i]) + ot.Observe(ct[i]) + ot.Expect(ct[i]) + } + if ot.Satisfied() { + t.Fatal("should not be satisfied before ExpectationsDone") + } + + ot.ExpectationsDone() + + if !ot.Satisfied() { + t.Fatal("should be satisfied") + } +} + // Verify that an expectation can be canceled before it's first expected. func Test_ObjectTracker_CancelBeforeExpect(t *testing.T) { ot := newObjTracker(schema.GroupVersionKind{}, nil) diff --git a/pkg/readiness/tracker_map.go b/pkg/readiness/tracker_map.go index f18fb86b932..57a596eaae1 100644 --- a/pkg/readiness/tracker_map.go +++ b/pkg/readiness/tracker_map.go @@ -66,6 +66,10 @@ func (t *trackerMap) Get(gvk schema.GroupVersionKind) Expectations { t.mu.Lock() defer t.mu.Unlock() + // re-retrieve map entry in case it was added after releasing the read lock. + if e, ok := t.m[gvk]; ok { + return e + } entry := newObjTracker(gvk, t.fn) t.m[gvk] = entry return entry