From b375037a42f48fd3f990816adebd8e4b3bfb7f25 Mon Sep 17 00:00:00 2001
From: John Tzikas <john.tzikas@mattermost.com>
Date: Wed, 25 Nov 2020 12:23:43 +0200
Subject: [PATCH] Syncronize access when updating config on search layer
 (#16393)

* Syncronize access when updating config on search layer

* Simplify validity of race condition on tests

* Add License header on new file

* Use atomic.Value for config access on search layer

* Apply PR suggestions
---
 store/searchlayer/layer.go      | 12 ++++++---
 store/searchlayer/layer_test.go | 45 +++++++++++++++++++++++++++++++++
 store/searchlayer/post_layer.go |  2 +-
 3 files changed, 55 insertions(+), 4 deletions(-)
 create mode 100644 store/searchlayer/layer_test.go

diff --git a/store/searchlayer/layer.go b/store/searchlayer/layer.go
index 5603dc3427..8544e6e464 100644
--- a/store/searchlayer/layer.go
+++ b/store/searchlayer/layer.go
@@ -4,6 +4,8 @@
 package searchlayer
 
 import (
+	"sync/atomic"
+
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/mattermost/mattermost-server/v5/services/searchengine"
@@ -17,15 +19,15 @@ type SearchStore struct {
 	team         *SearchTeamStore
 	channel      *SearchChannelStore
 	post         *SearchPostStore
-	config       *model.Config
+	configValue  atomic.Value
 }
 
 func NewSearchLayer(baseStore store.Store, searchEngine *searchengine.Broker, cfg *model.Config) *SearchStore {
 	searchStore := &SearchStore{
 		Store:        baseStore,
 		searchEngine: searchEngine,
-		config:       cfg,
 	}
+	searchStore.configValue.Store(cfg)
 	searchStore.channel = &SearchChannelStore{ChannelStore: baseStore.Channel(), rootStore: searchStore}
 	searchStore.post = &SearchPostStore{PostStore: baseStore.Post(), rootStore: searchStore}
 	searchStore.team = &SearchTeamStore{TeamStore: baseStore.Team(), rootStore: searchStore}
@@ -35,7 +37,11 @@ func NewSearchLayer(baseStore store.Store, searchEngine *searchengine.Broker, cf
 }
 
 func (s *SearchStore) UpdateConfig(cfg *model.Config) {
-	s.config = cfg
+	s.configValue.Store(cfg)
+}
+
+func (s *SearchStore) getConfig() *model.Config {
+	return s.configValue.Load().(*model.Config)
 }
 
 func (s *SearchStore) Channel() store.ChannelStore {
diff --git a/store/searchlayer/layer_test.go b/store/searchlayer/layer_test.go
new file mode 100644
index 0000000000..3a820a9a40
--- /dev/null
+++ b/store/searchlayer/layer_test.go
@@ -0,0 +1,45 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See LICENSE.txt for license information.
+
+package searchlayer_test
+
+import (
+	"os"
+	"sync"
+	"testing"
+
+	"github.com/mattermost/mattermost-server/v5/model"
+	"github.com/mattermost/mattermost-server/v5/services/searchengine"
+	"github.com/mattermost/mattermost-server/v5/store/searchlayer"
+	"github.com/mattermost/mattermost-server/v5/store/sqlstore"
+	"github.com/mattermost/mattermost-server/v5/store/storetest"
+	"github.com/mattermost/mattermost-server/v5/testlib"
+)
+
+// Test to verify race condition on UpdateConfig. The test must run with -race flag in order to verify
+// that there is no race. Ref: (#MM-30868)
+func TestUpdateConfigRace(t *testing.T) {
+	driverName := os.Getenv("MM_SQLSETTINGS_DRIVERNAME")
+	if driverName == "" {
+		driverName = model.DATABASE_DRIVER_POSTGRES
+	}
+	settings := storetest.MakeSqlSettings(driverName)
+	store := sqlstore.NewSqlSupplier(*settings, nil)
+
+	cfg := &model.Config{}
+	cfg.SetDefaults()
+	cfg.ClusterSettings.MaxIdleConns = model.NewInt(1)
+	searchEngine := searchengine.NewBroker(cfg, nil)
+	layer := searchlayer.NewSearchLayer(&testlib.TestStore{Store: store}, searchEngine, cfg)
+	var wg sync.WaitGroup
+
+	wg.Add(5)
+	for i := 0; i < 5; i++ {
+		go func() {
+			defer wg.Done()
+			layer.UpdateConfig(cfg.Clone())
+		}()
+	}
+
+	wg.Wait()
+}
diff --git a/store/searchlayer/post_layer.go b/store/searchlayer/post_layer.go
index 184fd8d157..ccddb6a310 100644
--- a/store/searchlayer/post_layer.go
+++ b/store/searchlayer/post_layer.go
@@ -186,7 +186,7 @@ func (s SearchPostStore) SearchPostsInTeamForUser(paramsList []*model.SearchPara
 		}
 	}
 
-	if *s.rootStore.config.SqlSettings.DisableDatabaseSearch {
+	if *s.rootStore.getConfig().SqlSettings.DisableDatabaseSearch {
 		mlog.Debug("Returning empty results for post SearchPostsInTeam as the database search is disabled")
 		return &model.PostSearchResults{PostList: model.NewPostList(), Matches: model.PostSearchMatches{}}, nil
 	}