fix race

Signed-off-by: Mikhail Scherba <[email protected]>

pkg/module_manager/models/modules/basic.go

@@ -56,8 +56,6 @@ type BasicModule struct {
 
 	valuesStorage *ValuesStorage
 
-	state *moduleState
-
 	hooks *HooksStorage
 
 	// dependency
@@ -66,6 +64,9 @@ type BasicModule struct {
 	keepTemporaryHookFiles bool
 
 	logger *log.Logger
+
+	l     *sync.RWMutex
+	state *moduleState
 }
 
 // TODO: add options WithLogger
@@ -92,6 +93,7 @@ func NewBasicModule(name, path string, order uint32, staticValues utils.Values,
 		},
 		hooks:                  newHooksStorage(),
 		keepTemporaryHookFiles: shapp.DebugKeepTmpFiles,
+		l:                      new(sync.RWMutex),
 	}
 
 	for _, opt := range opts {
@@ -186,11 +188,13 @@ func (bm *BasicModule) SetHooksControllersReady() {
 
 // ResetState drops the module state
 func (bm *BasicModule) ResetState() {
+	bm.l.Lock()
 	bm.state = &moduleState{
 		Phase:                Startup,
 		hookErrors:           make(map[string]error),
 		synchronizationState: NewSynchronizationState(),
 	}
+	bm.l.Unlock()
 }
 
 // RegisterHooks searches and registers all module hooks from a filesystem or GoHook Registry
@@ -471,30 +475,37 @@ func (bm *BasicModule) registerHooks(hks []*hooks.ModuleHook, logger *log.Logger
 
 // GetPhase ...
 func (bm *BasicModule) GetPhase() ModuleRunPhase {
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 	return bm.state.Phase
 }
 
 // SetPhase ...
 func (bm *BasicModule) SetPhase(phase ModuleRunPhase) {
+	bm.l.Lock()
 	bm.state.Phase = phase
+	bm.l.Unlock()
 }
 
 // SetError ...
 func (bm *BasicModule) SetError(err error) {
+	bm.l.Lock()
 	bm.state.lastModuleErr = err
+	bm.l.Unlock()
 }
 
 // SetStateEnabled ...
 func (bm *BasicModule) SetStateEnabled(e bool) {
+	bm.l.Lock()
 	bm.state.Enabled = e
+	bm.l.Unlock()
 }
 
 // SaveHookError ...
 func (bm *BasicModule) SaveHookError(hookName string, err error) {
-	bm.state.hookErrorsLock.Lock()
-	defer bm.state.hookErrorsLock.Unlock()
-
+	bm.l.Lock()
 	bm.state.hookErrors[hookName] = err
+	bm.l.Unlock()
 }
 
 // RunHooksByBinding gets all hooks for binding, for each hook it creates a BindingContext,
@@ -703,7 +714,9 @@ func (bm *BasicModule) RunEnabledScript(tmpDir string, precedingEnabledModules [
 	logEntry.Info("Enabled script run successful",
 		slog.Bool("result", moduleEnabled),
 		slog.String("status", result))
+	bm.l.Lock()
 	bm.state.enabledScriptResult = &moduleEnabled
+	bm.l.Unlock()
 	return moduleEnabled, nil
 }
 
@@ -1030,6 +1043,8 @@ func (bm *BasicModule) GetConfigValues(withPrefix bool) utils.Values {
 // Synchronization xxx
 // TODO: don't like this honestly, i think we can remake it
 func (bm *BasicModule) Synchronization() *SynchronizationState {
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 	return bm.state.synchronizationState
 }
 
@@ -1063,8 +1078,8 @@ func (bm *BasicModule) GetValuesPatches() []utils.ValuesPatch {
 
 // GetHookErrorsSummary get hooks errors summary report
 func (bm *BasicModule) GetHookErrorsSummary() string {
-	bm.state.hookErrorsLock.RLock()
-	defer bm.state.hookErrorsLock.RUnlock()
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 
 	hooksState := make([]string, 0, len(bm.state.hookErrors))
 	for name, err := range bm.state.hookErrors {
@@ -1082,13 +1097,15 @@ func (bm *BasicModule) GetHookErrorsSummary() string {
 
 // GetEnabledScriptResult returns a bool pointer to the enabled script results
 func (bm *BasicModule) GetEnabledScriptResult() *bool {
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 	return bm.state.enabledScriptResult
 }
 
 // GetLastHookError get error of the last executed hook
 func (bm *BasicModule) GetLastHookError() error {
-	bm.state.hookErrorsLock.RLock()
-	defer bm.state.hookErrorsLock.RUnlock()
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 
 	for name, err := range bm.state.hookErrors {
 		if err != nil {
@@ -1100,6 +1117,8 @@ func (bm *BasicModule) GetLastHookError() error {
 }
 
 func (bm *BasicModule) GetModuleError() error {
+	bm.l.RLock()
+	defer bm.l.RUnlock()
 	return bm.state.lastModuleErr
 }
 
@@ -1198,7 +1217,6 @@ type moduleState struct {
 	Phase                ModuleRunPhase
 	lastModuleErr        error
 	hookErrors           map[string]error
-	hookErrorsLock       sync.RWMutex
 	synchronizationState *SynchronizationState
 	enabledScriptResult  *bool
 }

pkg/module_manager/module_manager.go

@@ -116,9 +116,6 @@ type ModuleManager struct {
 	// Dependencies.
 	dependencies *ModuleManagerDependencies
 
-	// All known modules from specified directories ($MODULES_DIR)
-	modules *moduleset.ModulesSet
-
 	global *modules.GlobalModule
 
 	globalSynchronizationState *modules.SynchronizationState
@@ -136,6 +133,10 @@ type ModuleManager struct {
 	environmentManager *environmentmanager.Manager
 
 	logger *log.Logger
+
+	l *sync.Mutex
+	// All known modules from specified directories ($MODULES_DIR)
+	modules *moduleset.ModulesSet
 }
 
 var once sync.Once
@@ -168,6 +169,7 @@ func NewModuleManager(ctx context.Context, cfg *ModuleManagerConfig, logger *log
 		moduleScheduler: scheduler.NewScheduler(cctx, logger.Named("scheduler")),
 
 		logger: logger,
+		l:      new(sync.Mutex),
 	}
 
 	if len(cfg.DirectoryConfig.ChrootDir) > 0 {
@@ -1025,6 +1027,8 @@ func (mm *ModuleManager) PushRunModuleTask(moduleName string, doModuleStartup bo
 
 // AreModulesInited returns true if modulemanager's moduleset has already been initialized
 func (mm *ModuleManager) AreModulesInited() bool {
+	mm.l.Lock()
+	defer mm.l.Unlock()
 	return mm.modules.IsInited()
 }
 
@@ -1320,7 +1324,9 @@ func (mm *ModuleManager) registerModules(scriptEnabledExtender *script_extender.
 	log.Debug("Found modules",
 		slog.Any("modules", set.NamesInOrder()))
 
+	mm.l.Lock()
 	mm.modules = set
+	mm.l.Unlock()
 
 	return nil
 }