diff --git a/cmd/mesh-controller/composed_keys.go b/cmd/mesh-controller/composed_keys.go index 3bb5d6f4..8a934c69 100644 --- a/cmd/mesh-controller/composed_keys.go +++ b/cmd/mesh-controller/composed_keys.go @@ -355,11 +355,19 @@ const startTakeIns = 16 // which tries again every doneKeyRetry, loudly, so a failing module never holds up the look at the others. func takeDoneKeyAtStart(ctx context.Context, open *stores, module string) { mu, _ := doneKeysMu.LoadOrStore(module, &sync.Mutex{}) - mu.(*sync.Mutex).Lock() - err := takeDoneKeyOnce(ctx, open, module) + if !mu.(*sync.Mutex).TryLock() { + // A take-in already follows the module: told to look again, it takes what the start would have (issue 1945). + doneKeysLookAgain.Store(module, true) + return + } + doneKeysLookAgain.Delete(module) // what this look reads covers every wake before it + err := doneKeyTakeOnce(ctx, open, module) mu.(*sync.Mutex).Unlock() - if err != nil { - // A failure or a wait is followed by takeDoneKey, which says it and looks again (issue 1945). + // Read after letting go, as every holder of a module's lock does: a wake during this look only marked the module + // (the review of issue 1945). + _, again := doneKeysLookAgain.LoadAndDelete(module) + if err != nil || again { + // A failure, a wait or a wake meanwhile is followed by takeDoneKey, which says it and looks again. inBackground(func() { takeDoneKey(ctx, open, module) }) } } diff --git a/cmd/mesh-controller/composed_keys_waits_test.go b/cmd/mesh-controller/composed_keys_waits_test.go index f0760a43..839f9b4f 100644 --- a/cmd/mesh-controller/composed_keys_waits_test.go +++ b/cmd/mesh-controller/composed_keys_waits_test.go @@ -153,3 +153,32 @@ func TestAWakeJustBeforeTheFollowerLetsGoIsTaken(t *testing.T) { t.Fatalf("taken %d times: the wake just before the follower let go was lost", calls) } } + +// A wake during the start's take-in of a module is not lost: the start pass reads the mark once it lets go of the module, +// and hands the module to takeDoneKey, which takes it once more (the review of issue 1945: the third dropped wake of its +// kind). +func TestAWakeDuringTheStartsTakeInIsTaken(t *testing.T) { + was, wasRetry, wasSay := doneKeyTakeOnce, doneKeyRetry, doneKeysSay + t.Cleanup(func() { doneKeyTakeOnce, doneKeyRetry, doneKeysSay = was, wasRetry, wasSay }) + doneKeyRetry = 10 * time.Millisecond + doneKeysSay = func(string) {} + var mu sync.Mutex + calls := 0 + doneKeyTakeOnce = func(context.Context, *stores, string) error { + mu.Lock() + calls++ + n := calls + mu.Unlock() + if n == 1 { + takeDoneKey(context.Background(), nil, "mesh-host") // the newer key's wake, during the start's take-in + } + return nil + } + takeDoneKeyAtStart(context.Background(), nil, "mesh-host") + background.Wait() + mu.Lock() + defer mu.Unlock() + if calls != 2 { + t.Fatalf("taken %d times: the wake during the start's take-in was lost", calls) + } +}