Implement lazy initialization of module instances

It is correct fix for initialization order issue introduced in
https://github.com/emersion/maddy/pull/24.

config.go

@@ -15,9 +15,9 @@ Config matchers for module interfaces.
 */
 
 func authProvider(modName string) (module.AuthProvider, error) {
-	modObj := module.GetInstance(modName)
-	if modObj == nil {
-		return nil, fmt.Errorf("unknown auth. provider instance: %s", modName)
+	modObj, err := module.GetInstance(modName)
+	if err != nil {
+		return nil, err
 	}
 
 	provider, ok := modObj.(module.AuthProvider)
@@ -44,9 +44,9 @@ func authDirective(m *config.Map, node *config.Node) (interface{}, error) {
 }
 
 func storageBackend(modName string) (module.Storage, error) {
-	modObj := module.GetInstance(modName)
-	if modObj == nil {
-		return nil, fmt.Errorf("unknown storage backend instance: %s", modName)
+	modObj, err := module.GetInstance(modName)
+	if err != nil {
+		return nil, err
 	}
 
 	backend, ok := modObj.(module.Storage)
@@ -73,9 +73,9 @@ func storageDirective(m *config.Map, node *config.Node) (interface{}, error) {
 }
 
 func deliveryTarget(modName string) (module.DeliveryTarget, error) {
-	mod := module.GetInstance(modName)
+	mod, err := module.GetInstance(modName)
 	if mod == nil {
-		return nil, fmt.Errorf("unknown delivery target instance: %s", modName)
+		return nil, err
 	}
 
 	target, ok := mod.(module.DeliveryTarget)

dummy.go

@@ -37,5 +37,5 @@ func (d Dummy) Init(_ *config.Map) error {
 
 func init() {
 	module.Register("dummy", nil)
-	module.RegisterInstance(&Dummy{instName: "dummy"})
+	module.RegisterInstance(&Dummy{instName: "dummy"}, nil)
 }

maddy.go

@@ -45,7 +45,7 @@ func Start(cfg []config.Node) error {
 			return fmt.Errorf("%s:%d: unknown module: %s", block.File, block.Line, modName)
 		}
 
-		if mod := module.GetInstance(instName); mod != nil {
+		if mod := module.GetUninitedInstance(instName); mod != nil {
 			return fmt.Errorf("%s:%d: module instance named %s already exists", block.File, block.Line, instName)
 		}
 
@@ -55,14 +55,21 @@ func Start(cfg []config.Node) error {
 			return err
 		}
 
-		module.RegisterInstance(inst)
+		block := block
+		module.RegisterInstance(inst, config.NewMap(globals.Values, &block))
 		instances[instName] = modInfo{instance: inst, cfg: block}
 	}
 
-	addDefaultModule(instances, "default", createDefaultStorage, defaultStorageConfig)
-	addDefaultModule(instances, "default_remote_delivery", createDefaultRemoteDelivery, nil)
+	addDefaultModule(instances, "default", createDefaultStorage, globals.Values, defaultStorageConfig)
+	addDefaultModule(instances, "default_remote_delivery", createDefaultRemoteDelivery, globals.Values, nil)
 
 	for _, inst := range instances {
+		if module.Initialized[inst.instance.InstanceName()] {
+			log.Debugln("module init", inst.instance.Name(), inst.instance.InstanceName(), "skipped because it was lazily initialized before")
+			continue
+		}
+
+		module.Initialized[inst.instance.InstanceName()] = true
 		log.Debugln("module init", inst.instance.Name(), inst.instance.InstanceName())
 		if err := inst.instance.Init(config.NewMap(globals.Values, &inst.cfg)); err != nil {
 			return err
@@ -90,17 +97,17 @@ func Start(cfg []config.Node) error {
 	return nil
 }
 
-func addDefaultModule(insts map[string]modInfo, name string, factory func(string) (module.Module, error), cfgFactory func(string) config.Node) {
+func addDefaultModule(insts map[string]modInfo, name string, factory func(string) (module.Module, error), globalCfg map[string]interface{}, cfgFactory func(string) config.Node) {
 	if _, ok := insts[name]; !ok {
 		if mod, err := factory(name); err != nil {
 			log.Printf("failed to register %s: %v", name, err)
 		} else {
 			log.Debugf("module create %s %s (built-in)", mod.Name(), name)
-			module.RegisterInstance(mod)
 			info := modInfo{instance: mod}
 			if cfgFactory != nil {
 				info.cfg = cfgFactory(name)
 			}
+			module.RegisterInstance(mod, config.NewMap(globalCfg, &info.cfg))
 			insts[name] = info
 		}
 	} else {

module/instances.go

@@ -0,0 +1,69 @@
+package module
+
+import (
+	"fmt"
+
+	"github.com/emersion/maddy/config"
+	"github.com/emersion/maddy/log"
+)
+
+var (
+	Instances = make(map[string]struct {
+		mod Module
+		cfg *config.Map
+	})
+
+	Initialized = make(map[string]bool)
+)
+
+// Register adds module instance to the global registry.
+//
+// Instnace name must be unique. Second RegisterInstance with same instance
+// name will replace previous.
+func RegisterInstance(inst Module, cfg *config.Map) {
+	Instances[inst.InstanceName()] = struct {
+		mod Module
+		cfg *config.Map
+	}{inst, cfg}
+}
+
+// GetUninitedInstance returns module instance from global registry.
+//
+// Nil is returned if no module instance with specified name is Instances.
+//
+// Note that this function may return uninitialized module. If you need to ensure
+// that it is safe to interact with module instance - use GetInstance instead.
+func GetUninitedInstance(name string) Module {
+	mod, ok := Instances[name]
+	if !ok {
+		return nil
+	}
+
+	return mod.mod
+}
+
+// GetInstance returns module instance from global registry, initializing it if
+// necessary.
+//
+// Error is returned if module initialization fails or module instance does not
+// exists.
+func GetInstance(name string) (Module, error) {
+	mod, ok := Instances[name]
+	if !ok {
+		return nil, fmt.Errorf("unknown module instance: %s", name)
+	}
+
+	// Break circular dependencies.
+	if Initialized[name] {
+		log.Debugln("module init", mod.mod.Name(), name, "(lazy init skipped)")
+		return mod.mod, nil
+	}
+
+	log.Debugln("module init", mod.mod.Name(), name, "(lazy)")
+	Initialized[name] = true
+	if err := mod.mod.Init(mod.cfg); err != nil {
+		return mod.mod, err
+	}
+
+	return mod.mod, nil
+}

module/registry.go

@@ -33,36 +33,3 @@ func Get(name string) FuncNewModule {
 
 	return modules[name]
 }
-
-var instances = make(map[string]Module)
-var instancesLck sync.RWMutex
-
-// Register adds module instance to global registry.
-//
-// Instnace name must be unique. Second RegisterInstance with same instance
-// name will replace previous.
-func RegisterInstance(inst Module) {
-	instancesLck.Lock()
-	defer instancesLck.Unlock()
-
-	instances[inst.InstanceName()] = inst
-}
-
-// GetInstance returns module instance from global registry.
-//
-// Nil is returned if no module instance with specified name is registered.
-func GetInstance(name string) Module {
-	instancesLck.RLock()
-	defer instancesLck.RUnlock()
-
-	return instances[name]
-}
-
-// UnregisterInstance undoes effect of RegisterInstance, removing instance from
-// global registry.
-func UnregisterInstance(name string) {
-	instancesLck.Lock()
-	defer instancesLck.Unlock()
-
-	delete(instances, name)
-}

smtppipeline.go

@@ -181,8 +181,8 @@ func filterStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 		return nil, errors.New("filter: expected at least one argument")
 	}
 
-	modInst := module.GetInstance(node.Args[0])
-	if modInst == nil {
+	modInst, err := module.GetInstance(node.Args[0])
+	if err != nil {
 		return nil, fmt.Errorf("filter: unknown module instance: %s", node.Args[0])
 	}
 
@@ -202,8 +202,8 @@ func deliveryStepFromCfg(node config.Node) (SMTPPipelineStep, error) {
 		return nil, errors.New("delivery: expected at least one argument")
 	}
 
-	modInst := module.GetInstance(node.Args[0])
-	if modInst == nil {
+	modInst, err := module.GetInstance(node.Args[0])
+	if err != nil {
 		return nil, fmt.Errorf("delivery: unknown module instance: %s", node.Args[0])
 	}