From 15eac839236309cebda7c01b08bf833ac5e2fd22 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 5 Jul 2023 20:36:14 +0530 Subject: [PATCH] MM-52646: Fix data race on product notices (#23910) The UpdateProductNotices method runs in a separate goroutine which modifies a global structure called noticesCache. This creates race condition when one test finishes but the goroutine hasn't finished running. To fix this, we simply prevent the spawning of the goroutine by disabling the feature in the config. Ideally, we'd not have global state in the first place. But that's a separate matter. https://mattermost.atlassian.net/browse/MM-52646 ```release-note NONE ``` --- server/channels/app/server.go | 4 ++-- server/channels/app/server_test.go | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/server/channels/app/server.go b/server/channels/app/server.go index c6222afc2a..a70eca0459 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -462,12 +462,12 @@ func NewServer(options ...Option) (*Server, error) { // if enabled - perform initial product notices fetch if *s.platform.Config().AnnouncementSettings.AdminNoticesEnabled || *s.platform.Config().AnnouncementSettings.UserNoticesEnabled { - go func() { + s.platform.Go(func() { appInstance := New(ServerConnector(s.Channels())) if err := appInstance.UpdateProductNotices(); err != nil { mlog.Warn("Failed to perform initial product notices fetch", mlog.Err(err)) } - }() + }) } if s.skipPostInit { diff --git a/server/channels/app/server_test.go b/server/channels/app/server_test.go index a816c54de7..ec08ddd1ad 100644 --- a/server/channels/app/server_test.go +++ b/server/channels/app/server_test.go @@ -42,6 +42,8 @@ func newServerWithConfig(t *testing.T, f func(cfg *model.Config)) (*Server, erro store, err := config.NewStoreFromBacking(configStore, nil, false) require.NoError(t, err) cfg := store.Get() + *cfg.AnnouncementSettings.AdminNoticesEnabled = false + *cfg.AnnouncementSettings.UserNoticesEnabled = false cfg.SqlSettings = *mainHelper.GetSQLSettings() f(cfg) @@ -110,6 +112,8 @@ func TestStartServerNoS3Bucket(t *testing.T) { AmazonS3SSL: model.NewBool(false), } *cfg.ServiceSettings.ListenAddress = "localhost:0" + *cfg.AnnouncementSettings.AdminNoticesEnabled = false + *cfg.AnnouncementSettings.UserNoticesEnabled = false cfg.SqlSettings = *mainHelper.GetSQLSettings() _, _, err := store.Set(cfg) require.NoError(t, err) @@ -191,6 +195,8 @@ func TestStartServerTLSVersion(t *testing.T) { cfg := store.Get() testDir, _ := fileutils.FindDir("tests") + *cfg.AnnouncementSettings.AdminNoticesEnabled = false + *cfg.AnnouncementSettings.UserNoticesEnabled = false *cfg.ServiceSettings.ListenAddress = "localhost:0" *cfg.ServiceSettings.ConnectionSecurity = "TLS" *cfg.ServiceSettings.TLSMinVer = "1.2"