From 06ad69a406dc561d277ec0b0cb32112afee1ffa4 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 17 Nov 2022 20:44:18 +0530 Subject: [PATCH] MM-47465: Fix flaky TestDeleteChannel (#21686) Creating the post in a separate goroutine would create a race condition because that method also calls GetChannel. This can cause a bug if the cache was wiped before the the other goroutine would get a chance to update the cache. And if that happens, then it would populate the cache with the old value. Pseudo-code ``` func deleteChannel() { go func() { getFromCache() }() updateDB() wipeCache() // a } func getFromCache() { err := checkCache() if err == ErrNotFound { getFromDB() // x updateCache() // y } } func Test() { deleteChannel() getFromCache() } ``` If the sequence of events happen like - x - a - y Then the getFromCache() call later will get the wrong value from cache. The fix is to make the call synchronous. https://mattermost.atlassian.net/browse/MM-47465 Special thanks to @noxer for finding the root cause. ```release-note NONE ``` --- api4/channel_test.go | 1 - app/channel.go | 15 ++++++--------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/api4/channel_test.go b/api4/channel_test.go index 1e1ea4b2e1..16e432bf0d 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -1779,7 +1779,6 @@ func TestSearchGroupChannels(t *testing.T) { } func TestDeleteChannel(t *testing.T) { - t.Skip("MM-47465") th := Setup(t).InitBasic() defer th.TearDown() c := th.Client diff --git a/app/channel.go b/app/channel.go index 6bbe699fb0..f5c864ca38 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1420,13 +1420,10 @@ func (a *App) DeleteChannel(c request.CTX, channel *model.Channel, userID string c.Logger().Warn("Failed to post archive message", mlog.Err(err)) } } else { - a.Srv().Go(func() { - systemBot, err := a.GetSystemBot() - if err != nil { - c.Logger().Error("Failed to post archive message", mlog.Err(err)) - return - } - + systemBot, err := a.GetSystemBot() + if err != nil { + c.Logger().Warn("Failed to post archive message", mlog.Err(err)) + } else { post := &model.Post{ ChannelId: channel.Id, Message: fmt.Sprintf(i18n.T("api.channel.delete_channel.archived"), systemBot.Username), @@ -1438,9 +1435,9 @@ func (a *App) DeleteChannel(c request.CTX, channel *model.Channel, userID string } if _, err := a.CreatePost(c, post, channel, false, true); err != nil { - c.Logger().Error("Failed to post archive message", mlog.Err(err)) + c.Logger().Warn("Failed to post archive message", mlog.Err(err)) } - }) + } } now := model.GetMillis()