From a41b5a5234a2c121925e9c3050cec44f6d6b9bda Mon Sep 17 00:00:00 2001 From: Ivan Milchev Date: Wed, 17 Apr 2024 19:23:02 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20fix=20provider=20shutdown=20race?= =?UTF-8?q?=20condition=20(#3775)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🐛 fix provider shutdown race condition Signed-off-by: Ivan Milchev * fix tests Signed-off-by: Ivan Milchev --------- Signed-off-by: Ivan Milchev --- providers/coordinator.go | 12 ++---------- providers/coordinator_test.go | 9 +++++---- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/providers/coordinator.go b/providers/coordinator.go index c6ee7a7480..880323aab7 100644 --- a/providers/coordinator.go +++ b/providers/coordinator.go @@ -203,17 +203,9 @@ func (c *coordinator) RemoveRuntime(runtime *Runtime) { } } - // Analyze the providers that are still being used by active runtimes - usedProviders := map[string]struct{}{} - for _, r := range c.runtimes { - for _, p := range r.providers { - usedProviders[p.Instance.ID] = struct{}{} - } - } - // Shutdown any providers that are not being used anymore - for id, p := range c.runningByID { - if _, ok := usedProviders[id]; !ok { + if len(c.runtimes) == 0 { + for _, p := range c.runningByID { log.Debug().Msg("shutting down unused provider " + p.Name) if err := c.stop(p); err != nil { log.Warn().Err(err).Str("provider", p.Name).Msg("failed to shut down provider") diff --git a/providers/coordinator_test.go b/providers/coordinator_test.go index a6b3cd3728..70b0770034 100644 --- a/providers/coordinator_test.go +++ b/providers/coordinator_test.go @@ -113,6 +113,7 @@ func TestRemoveRuntime_StopUnusedProvider(t *testing.T) { // Setup another provider with another runtime mockPlugin2 := NewMockProviderPlugin(ctrl) + mockPlugin2.EXPECT().Shutdown(gomock.Any()).Times(1).Return(nil, nil) p2 := &RunningProvider{ ID: "provider2", Plugin: mockPlugin2, @@ -140,12 +141,12 @@ func TestRemoveRuntime_StopUnusedProvider(t *testing.T) { }, } - // Remove the first runtime + // Remove all runtimes c.RemoveRuntime(r1) + c.RemoveRuntime(r2) - // Verify that the first provider is stopped - assert.NotContains(t, c.runningByID, "provider1") - assert.Contains(t, c.runningByID, "provider2") + // Verify that all provider are stopped + assert.Empty(t, c.runningByID) } func TestRemoveRuntime_UsedProvider(t *testing.T) {