From ef07ad16c4cdf64772e6ee5b650aca13e65db262 Mon Sep 17 00:00:00 2001 From: Mikhail Scherba Date: Mon, 17 Feb 2025 16:53:46 +0300 Subject: [PATCH] fix race Signed-off-by: Mikhail Scherba --- pkg/module_manager/models/modules/basic.go | 38 ++++++++++++++++------ pkg/module_manager/module_manager.go | 12 +++++-- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/pkg/module_manager/models/modules/basic.go b/pkg/module_manager/models/modules/basic.go index 4356cec6..c003d618 100644 --- a/pkg/module_manager/models/modules/basic.go +++ b/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 } diff --git a/pkg/module_manager/module_manager.go b/pkg/module_manager/module_manager.go index f6c7ba5b..47ce9ddd 100644 --- a/pkg/module_manager/module_manager.go +++ b/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 }