From 1f0c061ce30f218e63fcc17e0fdfc8b90d754ba5 Mon Sep 17 00:00:00 2001
From: Matthew Holt <mholt@users.noreply.github.com>
Date: Thu, 16 May 2019 16:05:38 -0600
Subject: [PATCH] Architectural shift to using context for config and module
 state

---
 caddy.go                                   | 275 ++++++++-------------
 context.go                                 | 126 ++++++++++
 listeners.go                               |  33 ++-
 modules.go                                 |  95 +------
 modules/caddyhttp/caddyhttp.go             |  55 +++--
 modules/caddyhttp/reverseproxy/module.go   |  33 ---
 modules/caddyhttp/reverseproxy/upstream.go |   4 +-
 modules/caddyhttp/routes.go                |   8 +-
 modules/caddytls/acmemanager.go            |  10 +-
 modules/caddytls/connpolicy.go             |  10 +-
 modules/caddytls/tls.go                    |  23 +-
 11 files changed, 313 insertions(+), 359 deletions(-)
 create mode 100644 context.go

diff --git a/caddy.go b/caddy.go
index 585b9df8e..082c6991a 100644
--- a/caddy.go
+++ b/caddy.go
@@ -1,127 +1,118 @@
 package caddy2
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"log"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/mholt/certmagic"
 )
 
 // Run runs Caddy with the given config.
-func Run(cfg *Config) error {
-	// allow only one call to Start at a time,
-	// since various calls to LoadModule()
-	// access shared map moduleInstances
-	startMu.Lock()
-	defer startMu.Unlock()
-
-	// because we will need to roll back any state
-	// modifications if this function errors, we
-	// keep a single error value and scope all
-	// sub-operations to their own functions to
-	// ensure this error value does not get
-	// overridden or missed when it should have
-	// been set by a short assignment
-	var err error
-
-	// prepare the new config for use
-	cfg.apps = make(map[string]App)
-	cfg.moduleStates = make(map[string]interface{})
-
-	// reset the shared moduleInstances map; but
-	// keep a temporary reference to the current
-	// one so we can transfer over any necessary
-	// state to the new modules or to roll back
-	// if necessary
-	oldModuleInstances := moduleInstances
-	defer func() {
-		if err != nil {
-			moduleInstances = oldModuleInstances
-		}
-	}()
-	moduleInstances = make(map[string][]interface{})
-
-	// set up storage and make it CertMagic's default storage, too
-	err = func() error {
-		if cfg.StorageRaw != nil {
-			val, err := LoadModuleInline("system", "caddy.storage", cfg.StorageRaw)
-			if err != nil {
-				return fmt.Errorf("loading storage module: %v", err)
-			}
-			stor, err := val.(StorageConverter).CertMagicStorage()
-			if err != nil {
-				return fmt.Errorf("creating storage value: %v", err)
-			}
-			cfg.storage = stor
-			cfg.StorageRaw = nil // allow GC to deallocate - TODO: Does this help?
-		}
-		if cfg.storage == nil {
-			cfg.storage = &certmagic.FileStorage{Path: dataDir()}
-		}
-		certmagic.Default.Storage = cfg.storage
-
-		return nil
-	}()
-	if err != nil {
-		return err
-	}
-
-	// Load, Provision, Validate
-	err = func() error {
-		for modName, rawMsg := range cfg.AppsRaw {
-			val, err := LoadModule(modName, rawMsg)
-			if err != nil {
-				return fmt.Errorf("loading app module '%s': %v", modName, err)
-			}
-			cfg.apps[modName] = val.(App)
-		}
-		return nil
-	}()
-	if err != nil {
-		return err
-	}
-
-	// swap old config with the new one, and
-	// roll back this change if anything fails
+func Run(newCfg *Config) error {
 	currentCfgMu.Lock()
-	oldCfg := currentCfg
-	currentCfg = cfg
-	currentCfgMu.Unlock()
-	defer func() {
-		if err != nil {
-			currentCfgMu.Lock()
-			currentCfg = oldCfg
-			currentCfgMu.Unlock()
-		}
-	}()
+	defer currentCfgMu.Unlock()
 
-	// Start
-	err = func() error {
-		h := Handle{cfg}
-		for name, a := range cfg.apps {
-			err := a.Start(h)
+	if newCfg != nil {
+		// because we will need to roll back any state
+		// modifications if this function errors, we
+		// keep a single error value and scope all
+		// sub-operations to their own functions to
+		// ensure this error value does not get
+		// overridden or missed when it should have
+		// been set by a short assignment
+		var err error
+
+		// prepare the new config for use
+		newCfg.apps = make(map[string]App)
+
+		// create a context within which to load
+		// modules - essentially our new config's
+		// execution environment; be sure that
+		// cleanup occurs when we return if there
+		// was an error; otherwise, it will get
+		// cleaned up on next config cycle
+		ctx, cancel := NewContext(Context{Context: context.Background(), cfg: newCfg})
+		defer func() {
 			if err != nil {
-				for otherAppName, otherApp := range cfg.apps {
-					err := otherApp.Stop()
-					if err != nil {
-						log.Printf("aborting app %s: %v", otherAppName, err)
-					}
-				}
-				return fmt.Errorf("%s app module: start: %v", name, err)
+				cancel() // clean up now
 			}
+		}()
+		newCfg.cancelFunc = cancel // clean up later
+
+		// set up storage and make it CertMagic's default storage, too
+		err = func() error {
+			if newCfg.StorageRaw != nil {
+				val, err := ctx.LoadModuleInline("system", "caddy.storage", newCfg.StorageRaw)
+				if err != nil {
+					return fmt.Errorf("loading storage module: %v", err)
+				}
+				stor, err := val.(StorageConverter).CertMagicStorage()
+				if err != nil {
+					return fmt.Errorf("creating storage value: %v", err)
+				}
+				newCfg.storage = stor
+				newCfg.StorageRaw = nil // allow GC to deallocate - TODO: Does this help?
+			}
+			if newCfg.storage == nil {
+				newCfg.storage = &certmagic.FileStorage{Path: dataDir()}
+			}
+			certmagic.Default.Storage = newCfg.storage
+
+			return nil
+		}()
+		if err != nil {
+			return err
+		}
+
+		// Load, Provision, Validate
+		err = func() error {
+			for modName, rawMsg := range newCfg.AppsRaw {
+				val, err := ctx.LoadModule(modName, rawMsg)
+				if err != nil {
+					return fmt.Errorf("loading app module '%s': %v", modName, err)
+				}
+				newCfg.apps[modName] = val.(App)
+			}
+			return nil
+		}()
+		if err != nil {
+			return err
+		}
+
+		// Start
+		err = func() error {
+			var started []string
+			for name, a := range newCfg.apps {
+				err := a.Start()
+				if err != nil {
+					for _, otherAppName := range started {
+						err2 := newCfg.apps[otherAppName].Stop()
+						if err2 != nil {
+							err = fmt.Errorf("%v; additionally, aborting app %s: %v",
+								err, otherAppName, err2)
+						}
+					}
+					return fmt.Errorf("%s app module: start: %v", name, err)
+				}
+				started = append(started, name)
+			}
+			return nil
+		}()
+		if err != nil {
+			return err
 		}
-		return nil
-	}()
-	if err != nil {
-		return err
 	}
 
-	// Stop
+	// swap old config with the new one
+	oldCfg := currentCfg
+	currentCfg = newCfg
+
+	// Stop, Cleanup
 	if oldCfg != nil {
 		for name, a := range oldCfg.apps {
 			err := a.Stop()
@@ -129,26 +120,9 @@ func Run(cfg *Config) error {
 				log.Printf("[ERROR] stop %s: %v", name, err)
 			}
 		}
-	}
 
-	// shut down listeners that are no longer being used
-	err = func() error {
-		listenersMu.Lock()
-		for key, info := range listeners {
-			if atomic.LoadInt32(&info.usage) == 0 {
-				err := info.ln.Close()
-				if err != nil {
-					log.Printf("[ERROR] closing listener %s: %v", info.ln.Addr(), err)
-					continue
-				}
-				delete(listeners, key)
-			}
-		}
-		listenersMu.Unlock()
-		return nil
-	}()
-	if err != nil {
-		return err
+		// clean up old modules
+		oldCfg.cancelFunc()
 	}
 
 	return nil
@@ -156,7 +130,7 @@ func Run(cfg *Config) error {
 
 // App is a thing that Caddy runs.
 type App interface {
-	Start(Handle) error
+	Start() error
 	Stop() error
 }
 
@@ -172,46 +146,7 @@ type Config struct {
 	// keyed by module name.
 	apps map[string]App
 
-	// moduleStates stores the optional "global" state
-	// values of every module used by this configuration,
-	// keyed by module name.
-	moduleStates map[string]interface{}
-}
-
-// Handle allows app modules to access
-// the top-level Config in a controlled
-// manner without needing to rely on
-// global state.
-type Handle struct {
-	current *Config
-}
-
-// App returns the configured app named name. If no app with
-// that name is currently configured, a new empty one will be
-// instantiated. (The app module must still be registered.)
-func (h Handle) App(name string) (interface{}, error) {
-	if app, ok := h.current.apps[name]; ok {
-		return app, nil
-	}
-	modVal, err := LoadModule(name, nil)
-	if err != nil {
-		return nil, fmt.Errorf("instantiating new module %s: %v", name, err)
-	}
-	h.current.apps[name] = modVal.(App)
-	return modVal, nil
-}
-
-// GetStorage returns the configured Caddy storage implementation.
-// If no storage implementation is explicitly configured, the
-// default one is returned instead, as long as there is a current
-// configuration loaded.
-func GetStorage() certmagic.Storage {
-	currentCfgMu.RLock()
-	defer currentCfgMu.RUnlock()
-	if currentCfg == nil {
-		return nil
-	}
-	return currentCfg.storage
+	cancelFunc context.CancelFunc
 }
 
 // Duration is a JSON-string-unmarshable duration type.
@@ -236,17 +171,3 @@ var (
 	currentCfg   *Config
 	currentCfgMu sync.RWMutex
 )
-
-// moduleInstances stores the individual instantiated
-// values of modules, keyed by module name. The list
-// of instances of each module get passed into the
-// respective module's OnLoad callback, so they can
-// set up any global state and/or make sure their
-// configuration, when viewed as a whole, is valid.
-// Since this list is shared, only one Start() routine
-// must be allowed to happen at any given time.
-var moduleInstances = make(map[string][]interface{})
-
-// startMu ensures that only one Start() happens at a time.
-// This is important since moduleInstances is shared state.
-var startMu sync.Mutex
diff --git a/context.go b/context.go
new file mode 100644
index 000000000..ca2f44cee
--- /dev/null
+++ b/context.go
@@ -0,0 +1,126 @@
+package caddy2
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"log"
+	"reflect"
+
+	"github.com/mholt/certmagic"
+)
+
+type Context struct {
+	context.Context
+	moduleInstances map[string][]interface{}
+	cfg             *Config
+}
+
+func NewContext(ctx Context) (Context, context.CancelFunc) {
+	newCtx := Context{moduleInstances: make(map[string][]interface{}), cfg: ctx.cfg}
+	c, cancel := context.WithCancel(ctx.Context)
+	wrappedCancel := func() {
+		cancel()
+		for modName, modInstances := range newCtx.moduleInstances {
+			for _, inst := range modInstances {
+				if cu, ok := inst.(CleanerUpper); ok {
+					err := cu.Cleanup()
+					if err != nil {
+						log.Printf("[ERROR] %s (%p): cleanup: %v", modName, inst, err)
+					}
+				}
+			}
+		}
+	}
+	newCtx.Context = c
+	return newCtx, wrappedCancel
+}
+
+func (ctx Context) LoadModule(name string, rawMsg json.RawMessage) (interface{}, error) {
+	modulesMu.Lock()
+	mod, ok := modules[name]
+	modulesMu.Unlock()
+	if !ok {
+		return nil, fmt.Errorf("unknown module: %s", name)
+	}
+
+	if mod.New == nil {
+		return nil, fmt.Errorf("module '%s' has no constructor", mod.Name)
+	}
+
+	val, err := mod.New()
+	if err != nil {
+		return nil, fmt.Errorf("initializing module '%s': %v", mod.Name, err)
+	}
+
+	// value must be a pointer for unmarshaling into concrete type
+	if rv := reflect.ValueOf(val); rv.Kind() != reflect.Ptr {
+		val = reflect.New(rv.Type()).Elem().Addr().Interface()
+	}
+
+	// fill in its config only if there is a config to fill in
+	if len(rawMsg) > 0 {
+		err = json.Unmarshal(rawMsg, &val)
+		if err != nil {
+			return nil, fmt.Errorf("decoding module config: %s: %v", mod.Name, err)
+		}
+	}
+
+	if prov, ok := val.(Provisioner); ok {
+		err := prov.Provision(ctx)
+		if err != nil {
+			return nil, fmt.Errorf("provision %s: %v", mod.Name, err)
+		}
+	}
+
+	if validator, ok := val.(Validator); ok {
+		err := validator.Validate(ctx)
+		if err != nil {
+			if cleanerUpper, ok := val.(CleanerUpper); ok {
+				err2 := cleanerUpper.Cleanup()
+				if err2 != nil {
+					err = fmt.Errorf("%v; additionally, cleanup: %v", err, err2)
+				}
+				return nil, fmt.Errorf("%s: invalid configuration: %v", mod.Name, err)
+			}
+		}
+	}
+
+	ctx.moduleInstances[name] = append(ctx.moduleInstances[name], val)
+
+	return val, nil
+}
+
+func (ctx Context) LoadModuleInline(moduleNameKey, moduleScope string, raw json.RawMessage) (interface{}, error) {
+	moduleName, err := getModuleNameInline(moduleNameKey, raw)
+	if err != nil {
+		return nil, err
+	}
+
+	val, err := ctx.LoadModule(moduleScope+"."+moduleName, raw)
+	if err != nil {
+		return nil, fmt.Errorf("loading module '%s': %v", moduleName, err)
+	}
+
+	return val, nil
+}
+
+// App returns the configured app named name. If no app with
+// that name is currently configured, a new empty one will be
+// instantiated. (The app module must still be registered.)
+func (ctx Context) App(name string) (interface{}, error) {
+	if app, ok := ctx.cfg.apps[name]; ok {
+		return app, nil
+	}
+	modVal, err := ctx.LoadModule(name, nil)
+	if err != nil {
+		return nil, fmt.Errorf("instantiating new module %s: %v", name, err)
+	}
+	ctx.cfg.apps[name] = modVal.(App)
+	return modVal, nil
+}
+
+// Storage returns the configured Caddy storage implementation.
+func (ctx Context) Storage() certmagic.Storage {
+	return ctx.cfg.storage
+}
diff --git a/listeners.go b/listeners.go
index db2ebaf56..31b81fc3a 100644
--- a/listeners.go
+++ b/listeners.go
@@ -9,6 +9,7 @@ import (
 )
 
 // Listen returns a listener suitable for use in a Caddy module.
+// Always be sure to close listeners when you are done with them.
 func Listen(network, addr string) (net.Listener, error) {
 	lnKey := network + "/" + addr
 
@@ -16,9 +17,9 @@ func Listen(network, addr string) (net.Listener, error) {
 	defer listenersMu.Unlock()
 
 	// if listener already exists, increment usage counter, then return listener
-	if lnInfo, ok := listeners[lnKey]; ok {
-		atomic.AddInt32(&lnInfo.usage, 1)
-		return &fakeCloseListener{usage: &lnInfo.usage, Listener: lnInfo.ln}, nil
+	if lnUsage, ok := listeners[lnKey]; ok {
+		atomic.AddInt32(&lnUsage.usage, 1)
+		return &fakeCloseListener{usage: &lnUsage.usage, key: lnKey, Listener: lnUsage.ln}, nil
 	}
 
 	// or, create new one and save it
@@ -28,10 +29,10 @@ func Listen(network, addr string) (net.Listener, error) {
 	}
 
 	// make sure to start its usage counter at 1
-	lnInfo := &listenerUsage{usage: 1, ln: ln}
-	listeners[lnKey] = lnInfo
+	lnUsage := &listenerUsage{usage: 1, ln: ln}
+	listeners[lnKey] = lnUsage
 
-	return &fakeCloseListener{usage: &lnInfo.usage, Listener: ln}, nil
+	return &fakeCloseListener{usage: &lnUsage.usage, key: lnKey, Listener: ln}, nil
 }
 
 // fakeCloseListener's Close() method is a no-op. This allows
@@ -42,6 +43,7 @@ func Listen(network, addr string) (net.Listener, error) {
 type fakeCloseListener struct {
 	closed int32  // accessed atomically
 	usage  *int32 // accessed atomically
+	key    string
 	net.Listener
 }
 
@@ -80,8 +82,9 @@ func (fcl *fakeCloseListener) Accept() (net.Conn, error) {
 	return nil, err
 }
 
-// Close stops accepting new connections, but does not
-// actually close the underlying listener.
+// Close stops accepting new connections without
+// closing the underlying listener, unless no one
+// else is using it.
 func (fcl *fakeCloseListener) Close() error {
 	if atomic.CompareAndSwapInt32(&fcl.closed, 0, 1) {
 		// unfortunately, there is no way to cancel any
@@ -99,8 +102,18 @@ func (fcl *fakeCloseListener) Close() error {
 		}
 
 		// since we're no longer using this listener,
-		// decrement the usage counter
-		atomic.AddInt32(fcl.usage, -1)
+		// decrement the usage counter and, if no one
+		// else is using it, close underlying listener
+		if atomic.AddInt32(fcl.usage, -1) == 0 {
+			listenersMu.Lock()
+			delete(listeners, fcl.key)
+			listenersMu.Unlock()
+			err := fcl.Listener.Close()
+			if err != nil {
+				return err
+			}
+		}
+
 	}
 
 	return nil
diff --git a/modules.go b/modules.go
index b2c9b96ff..bba6b93a4 100644
--- a/modules.go
+++ b/modules.go
@@ -3,7 +3,6 @@ package caddy2
 import (
 	"encoding/json"
 	"fmt"
-	"reflect"
 	"sort"
 	"strings"
 	"sync"
@@ -32,9 +31,6 @@ func RegisterModule(mod Module) error {
 	if mod.Name == "caddy" {
 		return fmt.Errorf("modules cannot be named 'caddy'")
 	}
-	if strings.HasPrefix(mod.Name, "caddy.") {
-		return fmt.Errorf("modules cannot be namespaced in 'caddy'")
-	}
 
 	modulesMu.Lock()
 	defer modulesMu.Unlock()
@@ -123,88 +119,6 @@ func Modules() []string {
 	return names
 }
 
-// LoadModule decodes rawMsg into a new instance of mod and
-// returns the value. If mod.New() does not return a pointer
-// value, it is converted to one so that it is unmarshaled
-// into the underlying concrete type. If mod.New is nil, an
-// error is returned. If the module implements Validator or
-// Provisioner interfaces, those methods are invoked to
-// ensure the module is fully configured and valid before
-// being used.
-func LoadModule(name string, rawMsg json.RawMessage) (interface{}, error) {
-	modulesMu.Lock()
-	mod, ok := modules[name]
-	modulesMu.Unlock()
-	if !ok {
-		return nil, fmt.Errorf("unknown module: %s", name)
-	}
-
-	if mod.New == nil {
-		return nil, fmt.Errorf("module '%s' has no constructor", mod.Name)
-	}
-
-	val, err := mod.New()
-	if err != nil {
-		return nil, fmt.Errorf("initializing module '%s': %v", mod.Name, err)
-	}
-
-	// value must be a pointer for unmarshaling into concrete type
-	if rv := reflect.ValueOf(val); rv.Kind() != reflect.Ptr {
-		val = reflect.New(rv.Type()).Elem().Addr().Interface()
-	}
-
-	// fill in its config only if there is a config to fill in
-	if len(rawMsg) > 0 {
-		err = json.Unmarshal(rawMsg, &val)
-		if err != nil {
-			return nil, fmt.Errorf("decoding module config: %s: %v", mod.Name, err)
-		}
-	}
-
-	if prov, ok := val.(Provisioner); ok {
-		err := prov.Provision()
-		if err != nil {
-			return nil, fmt.Errorf("provision %s: %v", mod.Name, err)
-		}
-	}
-
-	if validator, ok := val.(Validator); ok {
-		err := validator.Validate()
-		if err != nil {
-			return nil, fmt.Errorf("%s: invalid configuration: %v", mod.Name, err)
-		}
-	}
-
-	moduleInstances[mod.Name] = append(moduleInstances[mod.Name], val)
-
-	return val, nil
-}
-
-// LoadModuleInline loads a module from a JSON raw message which decodes
-// to a map[string]interface{}, where one of the keys is moduleNameKey
-// and the corresponding value is the module name as a string, which
-// can be found in the given scope.
-//
-// This allows modules to be decoded into their concrete types and
-// used when their names cannot be the unique key in a map, such as
-// when there are multiple instances in the map or it appears in an
-// array (where there are no custom keys). In other words, the key
-// containing the module name is treated special/separate from all
-// the other keys.
-func LoadModuleInline(moduleNameKey, moduleScope string, raw json.RawMessage) (interface{}, error) {
-	moduleName, err := getModuleNameInline(moduleNameKey, raw)
-	if err != nil {
-		return nil, err
-	}
-
-	val, err := LoadModule(moduleScope+"."+moduleName, raw)
-	if err != nil {
-		return nil, fmt.Errorf("loading module '%s': %v", moduleName, err)
-	}
-
-	return val, nil
-}
-
 // getModuleNameInline loads the string value from raw of moduleNameKey,
 // where raw must be a JSON encoding of a map.
 func getModuleNameInline(moduleNameKey string, raw json.RawMessage) (string, error) {
@@ -228,14 +142,19 @@ func getModuleNameInline(moduleNameKey string, raw json.RawMessage) (string, err
 // always be fast (imperceptible running time) and an error should
 // be returned only if the value's configuration is invalid.
 type Validator interface {
-	Validate() error
+	Validate(Context) error
 }
 
 // Provisioner is implemented by modules which may need to perform
 // some additional "setup" steps immediately after being loaded.
 // This method will be called after Validate() (if implemented).
 type Provisioner interface {
-	Provision() error
+	Provision(Context) error
+}
+
+// TODO: different name...
+type CleanerUpper interface {
+	Cleanup() error
 }
 
 var (
diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go
index dfb2ea076..0fe9c98c8 100644
--- a/modules/caddyhttp/caddyhttp.go
+++ b/modules/caddyhttp/caddyhttp.go
@@ -37,16 +37,20 @@ type App struct {
 	Servers     map[string]*Server `json:"servers"`
 
 	servers []*http.Server
+
+	ctx caddy2.Context
 }
 
 // Provision sets up the app.
-func (hc *App) Provision() error {
-	for _, srv := range hc.Servers {
-		err := srv.Routes.Provision()
+func (app *App) Provision(ctx caddy2.Context) error {
+	app.ctx = ctx
+
+	for _, srv := range app.Servers {
+		err := srv.Routes.Provision(ctx)
 		if err != nil {
 			return fmt.Errorf("setting up server routes: %v", err)
 		}
-		err = srv.Errors.Routes.Provision()
+		err = srv.Errors.Routes.Provision(ctx)
 		if err != nil {
 			return fmt.Errorf("setting up server error handling routes: %v", err)
 		}
@@ -56,10 +60,10 @@ func (hc *App) Provision() error {
 }
 
 // Validate ensures the app's configuration is valid.
-func (hc *App) Validate() error {
+func (app *App) Validate() error {
 	// each server must use distinct listener addresses
 	lnAddrs := make(map[string]string)
-	for srvName, srv := range hc.Servers {
+	for srvName, srv := range app.Servers {
 		for _, addr := range srv.Listen {
 			netw, expanded, err := parseListenAddr(addr)
 			if err != nil {
@@ -78,13 +82,13 @@ func (hc *App) Validate() error {
 }
 
 // Start runs the app. It sets up automatic HTTPS if enabled.
-func (hc *App) Start(handle caddy2.Handle) error {
-	err := hc.automaticHTTPS(handle)
+func (app *App) Start() error {
+	err := app.automaticHTTPS()
 	if err != nil {
 		return fmt.Errorf("enabling automatic HTTPS: %v", err)
 	}
 
-	for srvName, srv := range hc.Servers {
+	for srvName, srv := range app.Servers {
 		s := &http.Server{
 			ReadTimeout:       time.Duration(srv.ReadTimeout),
 			ReadHeaderTimeout: time.Duration(srv.ReadHeaderTimeout),
@@ -110,13 +114,13 @@ func (hc *App) Start(handle caddy2.Handle) error {
 				}
 
 				// enable TLS
-				httpPort := hc.HTTPPort
+				httpPort := app.HTTPPort
 				if httpPort == 0 {
 					httpPort = DefaultHTTPPort
 				}
 				_, port, _ := net.SplitHostPort(addr)
 				if len(srv.TLSConnPolicies) > 0 && port != strconv.Itoa(httpPort) {
-					tlsCfg, err := srv.TLSConnPolicies.TLSConfig(handle)
+					tlsCfg, err := srv.TLSConnPolicies.TLSConfig(app.ctx)
 					if err != nil {
 						return fmt.Errorf("%s/%s: making TLS configuration: %v", network, addr, err)
 					}
@@ -124,7 +128,7 @@ func (hc *App) Start(handle caddy2.Handle) error {
 				}
 
 				go s.Serve(ln)
-				hc.servers = append(hc.servers, s)
+				app.servers = append(app.servers, s)
 			}
 		}
 	}
@@ -133,14 +137,14 @@ func (hc *App) Start(handle caddy2.Handle) error {
 }
 
 // Stop gracefully shuts down the HTTP server.
-func (hc *App) Stop() error {
+func (app *App) Stop() error {
 	ctx := context.Background()
-	if hc.GracePeriod > 0 {
+	if app.GracePeriod > 0 {
 		var cancel context.CancelFunc
-		ctx, cancel = context.WithTimeout(ctx, time.Duration(hc.GracePeriod))
+		ctx, cancel = context.WithTimeout(ctx, time.Duration(app.GracePeriod))
 		defer cancel()
 	}
-	for _, s := range hc.servers {
+	for _, s := range app.servers {
 		err := s.Shutdown(ctx)
 		if err != nil {
 			return err
@@ -149,8 +153,8 @@ func (hc *App) Stop() error {
 	return nil
 }
 
-func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
-	tlsAppIface, err := handle.App("tls")
+func (app *App) automaticHTTPS() error {
+	tlsAppIface, err := app.ctx.App("tls")
 	if err != nil {
 		return fmt.Errorf("getting tls app: %v", err)
 	}
@@ -159,7 +163,7 @@ func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
 	lnAddrMap := make(map[string]struct{})
 	var redirRoutes RouteList
 
-	for srvName, srv := range hc.Servers {
+	for srvName, srv := range app.Servers {
 		srv.tlsApp = tlsApp
 
 		if srv.DisableAutoHTTPS {
@@ -209,7 +213,7 @@ func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
 				if err != nil {
 					return fmt.Errorf("%s: invalid listener address: %v", srvName, addr)
 				}
-				httpRedirLnAddr := joinListenAddr(netw, host, strconv.Itoa(hc.HTTPPort))
+				httpRedirLnAddr := joinListenAddr(netw, host, strconv.Itoa(app.HTTPPort))
 				lnAddrMap[httpRedirLnAddr] = struct{}{}
 
 				if parts := strings.SplitN(port, "-", 2); len(parts) == 2 {
@@ -217,7 +221,7 @@ func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
 				}
 				redirTo := "https://{request.host}"
 
-				httpsPort := hc.HTTPSPort
+				httpsPort := app.HTTPSPort
 				if httpsPort == 0 {
 					httpsPort = DefaultHTTPSPort
 				}
@@ -253,13 +257,13 @@ func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
 				continue
 			}
 			for _, a := range addrs {
-				if hc.listenerTaken(netw, a) {
+				if app.listenerTaken(netw, a) {
 					continue mapLoop
 				}
 			}
 			lnAddrs = append(lnAddrs, addr)
 		}
-		hc.Servers["auto_https_redirects"] = &Server{
+		app.Servers["auto_https_redirects"] = &Server{
 			Listen:           lnAddrs,
 			Routes:           redirRoutes,
 			DisableAutoHTTPS: true,
@@ -269,8 +273,8 @@ func (hc *App) automaticHTTPS(handle caddy2.Handle) error {
 	return nil
 }
 
-func (hc *App) listenerTaken(network, address string) bool {
-	for _, srv := range hc.Servers {
+func (app *App) listenerTaken(network, address string) bool {
+	for _, srv := range app.Servers {
 		for _, addr := range srv.Listen {
 			netw, addrs, err := parseListenAddr(addr)
 			if err != nil || netw != network {
@@ -491,3 +495,4 @@ const (
 
 // Interface guards
 var _ HTTPInterfaces = middlewareResponseWriter{}
+var _ caddy2.App = (*App)(nil)
diff --git a/modules/caddyhttp/reverseproxy/module.go b/modules/caddyhttp/reverseproxy/module.go
index cc53bf536..1bfc9ad50 100755
--- a/modules/caddyhttp/reverseproxy/module.go
+++ b/modules/caddyhttp/reverseproxy/module.go
@@ -1,8 +1,6 @@
 package reverseproxy
 
 import (
-	"fmt"
-
 	"bitbucket.org/lightcodelabs/caddy2"
 )
 
@@ -11,36 +9,5 @@ func init() {
 	caddy2.RegisterModule(caddy2.Module{
 		Name: "http.responders.reverse_proxy",
 		New:  func() (interface{}, error) { return new(LoadBalanced), nil },
-		OnLoad: func(instances []interface{}, _ interface{}) (interface{}, error) {
-			// we don't need to do anything with prior state because healthcheckers are
-			// cleaned up in OnUnload.
-			s := &State{
-				HealthCheckers: []*HealthChecker{},
-			}
-
-			for _, i := range instances {
-				lb := i.(*LoadBalanced)
-
-				err := NewLoadBalancedReverseProxy(lb, s)
-				if err != nil {
-					return nil, err
-				}
-			}
-
-			return s, nil
-		},
-		OnUnload: func(state interface{}) error {
-			s, ok := state.(*State)
-			if !ok {
-				return fmt.Errorf("proxy OnLoad: prior state not expected proxy.State type")
-			}
-
-			// cleanup old healthcheckers
-			for _, hc := range s.HealthCheckers {
-				hc.Stop()
-			}
-
-			return nil
-		},
 	})
 }
diff --git a/modules/caddyhttp/reverseproxy/upstream.go b/modules/caddyhttp/reverseproxy/upstream.go
index b521d46b6..7e429f9de 100755
--- a/modules/caddyhttp/reverseproxy/upstream.go
+++ b/modules/caddyhttp/reverseproxy/upstream.go
@@ -76,7 +76,7 @@ var (
 )
 
 // NewLoadBalancedReverseProxy returns a collection of Upstreams that are to be loadbalanced.
-func NewLoadBalancedReverseProxy(lb *LoadBalanced, state *State) error {
+func NewLoadBalancedReverseProxy(lb *LoadBalanced, state *State, ctx caddy2.Context) error {
 	// set defaults
 	if lb.NoHealthyUpstreamsMessage == "" {
 		lb.NoHealthyUpstreamsMessage = msgNoHealthyUpstreams
@@ -115,7 +115,7 @@ func NewLoadBalancedReverseProxy(lb *LoadBalanced, state *State) error {
 
 		if uc.CircuitBreaker != nil {
 			if _, err := caddy2.GetModule(cbModule); err == nil {
-				val, err := caddy2.LoadModule(cbModule, uc.CircuitBreaker)
+				val, err := ctx.LoadModule(cbModule, uc.CircuitBreaker)
 				if err == nil {
 					cbv, ok := val.(CircuitBreaker)
 					if ok {
diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go
index cd612ac72..d204939f9 100644
--- a/modules/caddyhttp/routes.go
+++ b/modules/caddyhttp/routes.go
@@ -29,11 +29,11 @@ type ServerRoute struct {
 type RouteList []ServerRoute
 
 // Provision sets up all the routes by loading the modules.
-func (routes RouteList) Provision() error {
+func (routes RouteList) Provision(ctx caddy2.Context) error {
 	for i, route := range routes {
 		// matchers
 		for modName, rawMsg := range route.Matchers {
-			val, err := caddy2.LoadModule("http.matchers."+modName, rawMsg)
+			val, err := ctx.LoadModule("http.matchers."+modName, rawMsg)
 			if err != nil {
 				return fmt.Errorf("loading matcher module '%s': %v", modName, err)
 			}
@@ -43,7 +43,7 @@ func (routes RouteList) Provision() error {
 
 		// middleware
 		for j, rawMsg := range route.Apply {
-			mid, err := caddy2.LoadModuleInline("middleware", "http.middleware", rawMsg)
+			mid, err := ctx.LoadModuleInline("middleware", "http.middleware", rawMsg)
 			if err != nil {
 				return fmt.Errorf("loading middleware module in position %d: %v", j, err)
 			}
@@ -53,7 +53,7 @@ func (routes RouteList) Provision() error {
 
 		// responder
 		if route.Respond != nil {
-			resp, err := caddy2.LoadModuleInline("responder", "http.responders", route.Respond)
+			resp, err := ctx.LoadModuleInline("responder", "http.responders", route.Respond)
 			if err != nil {
 				return fmt.Errorf("loading responder module: %v", err)
 			}
diff --git a/modules/caddytls/acmemanager.go b/modules/caddytls/acmemanager.go
index 40e2d248c..59fc7c36e 100644
--- a/modules/caddytls/acmemanager.go
+++ b/modules/caddytls/acmemanager.go
@@ -40,10 +40,10 @@ func (m *acmeManagerMaker) newManager(interactive bool) (certmagic.Manager, erro
 	return nil, nil
 }
 
-func (m *acmeManagerMaker) Provision() error {
+func (m *acmeManagerMaker) Provision(ctx caddy2.Context) error {
 	// DNS providers
 	if m.Challenges.DNS != nil {
-		val, err := caddy2.LoadModuleInline("provider", "tls.dns", m.Challenges.DNS)
+		val, err := ctx.LoadModuleInline("provider", "tls.dns", m.Challenges.DNS)
 		if err != nil {
 			return fmt.Errorf("loading TLS storage module: %s", err)
 		}
@@ -53,7 +53,7 @@ func (m *acmeManagerMaker) Provision() error {
 
 	// policy-specific storage implementation
 	if m.Storage != nil {
-		val, err := caddy2.LoadModuleInline("system", "caddy.storage", m.Storage)
+		val, err := ctx.LoadModuleInline("system", "caddy.storage", m.Storage)
 		if err != nil {
 			return fmt.Errorf("loading TLS storage module: %s", err)
 		}
@@ -93,10 +93,10 @@ func (m *acmeManagerMaker) setDefaults() {
 // makeCertMagicConfig converts m into a certmagic.Config, because
 // this is a special case where the default manager is the certmagic
 // Config and not a separate manager.
-func (m *acmeManagerMaker) makeCertMagicConfig() certmagic.Config {
+func (m *acmeManagerMaker) makeCertMagicConfig(ctx caddy2.Context) certmagic.Config {
 	storage := m.storage
 	if storage == nil {
-		storage = caddy2.GetStorage()
+		storage = ctx.Storage()
 	}
 
 	var ond *certmagic.OnDemandConfig
diff --git a/modules/caddytls/connpolicy.go b/modules/caddytls/connpolicy.go
index bdbd79f4b..45fe83a51 100644
--- a/modules/caddytls/connpolicy.go
+++ b/modules/caddytls/connpolicy.go
@@ -18,11 +18,11 @@ type ConnectionPolicies []*ConnectionPolicy
 // TLSConfig converts the group of policies to a standard-lib-compatible
 // TLS configuration which selects the first matching policy based on
 // the ClientHello.
-func (cp ConnectionPolicies) TLSConfig(handle caddy2.Handle) (*tls.Config, error) {
+func (cp ConnectionPolicies) TLSConfig(ctx caddy2.Context) (*tls.Config, error) {
 	// connection policy matchers
 	for i, pol := range cp {
 		for modName, rawMsg := range pol.MatchersRaw {
-			val, err := caddy2.LoadModule("tls.handshake_match."+modName, rawMsg)
+			val, err := ctx.LoadModule("tls.handshake_match."+modName, rawMsg)
 			if err != nil {
 				return nil, fmt.Errorf("loading handshake matcher module '%s': %s", modName, err)
 			}
@@ -33,7 +33,7 @@ func (cp ConnectionPolicies) TLSConfig(handle caddy2.Handle) (*tls.Config, error
 
 	// pre-build standard TLS configs so we don't have to at handshake-time
 	for i := range cp {
-		err := cp[i].buildStandardTLSConfig(handle)
+		err := cp[i].buildStandardTLSConfig(ctx)
 		if err != nil {
 			return nil, fmt.Errorf("connection policy %d: building standard TLS config: %s", i, err)
 		}
@@ -74,8 +74,8 @@ type ConnectionPolicy struct {
 	stdTLSConfig *tls.Config
 }
 
-func (cp *ConnectionPolicy) buildStandardTLSConfig(handle caddy2.Handle) error {
-	tlsAppIface, err := handle.App("tls")
+func (cp *ConnectionPolicy) buildStandardTLSConfig(ctx caddy2.Context) error {
+	tlsAppIface, err := ctx.App("tls")
 	if err != nil {
 		return fmt.Errorf("getting tls app: %v", err)
 	}
diff --git a/modules/caddytls/tls.go b/modules/caddytls/tls.go
index fbc850cd0..4743e6b51 100644
--- a/modules/caddytls/tls.go
+++ b/modules/caddytls/tls.go
@@ -26,10 +26,13 @@ type TLS struct {
 
 	certificateLoaders []CertificateLoader
 	certCache          *certmagic.Cache
+	ctx                caddy2.Context
 }
 
 // Provision sets up the configuration for the TLS app.
-func (t *TLS) Provision() error {
+func (t *TLS) Provision(ctx caddy2.Context) error {
+	t.ctx = ctx
+
 	// set up the certificate cache
 	// TODO: this makes a new cache every time; better to only make a new
 	// cache (or even better, add/remove only what is necessary) if the
@@ -41,7 +44,7 @@ func (t *TLS) Provision() error {
 	})
 
 	for i, ap := range t.Automation.Policies {
-		val, err := caddy2.LoadModuleInline("module", "tls.management", ap.Management)
+		val, err := ctx.LoadModuleInline("module", "tls.management", ap.Management)
 		if err != nil {
 			return fmt.Errorf("loading TLS automation management module: %s", err)
 		}
@@ -54,7 +57,7 @@ func (t *TLS) Provision() error {
 		if modName == automateKey {
 			continue // special case; these will be loaded in later
 		}
-		val, err := caddy2.LoadModule("tls.certificates."+modName, rawMsg)
+		val, err := ctx.LoadModule("tls.certificates."+modName, rawMsg)
 		if err != nil {
 			return fmt.Errorf("loading certificate module '%s': %s", modName, err)
 		}
@@ -65,7 +68,7 @@ func (t *TLS) Provision() error {
 }
 
 // Start activates the TLS module.
-func (t *TLS) Start(handle caddy2.Handle) error {
+func (t *TLS) Start() error {
 	// load manual/static (unmanaged) certificates
 	for _, loader := range t.certificateLoaders {
 		certs, err := loader.LoadCertificates()
@@ -73,7 +76,7 @@ func (t *TLS) Start(handle caddy2.Handle) error {
 			return fmt.Errorf("loading certificates: %v", err)
 		}
 		magic := certmagic.New(t.certCache, certmagic.Config{
-			Storage: caddy2.GetStorage(),
+			Storage: t.ctx.Storage(),
 		})
 		for _, cert := range certs {
 			err := magic.CacheUnmanagedTLSCertificate(cert)
@@ -114,7 +117,7 @@ func (t *TLS) Stop() error {
 func (t *TLS) Manage(names []string) error {
 	for _, name := range names {
 		ap := t.getAutomationPolicyForName(name)
-		magic := certmagic.New(t.certCache, ap.makeCertMagicConfig())
+		magic := certmagic.New(t.certCache, ap.makeCertMagicConfig(t.ctx))
 		err := magic.Manage([]string{name})
 		if err != nil {
 			return fmt.Errorf("automate: manage %s: %v", name, err)
@@ -130,13 +133,13 @@ func (t *TLS) HandleHTTPChallenge(w http.ResponseWriter, r *http.Request) bool {
 		return false
 	}
 	ap := t.getAutomationPolicyForName(r.Host)
-	magic := certmagic.New(t.certCache, ap.makeCertMagicConfig())
+	magic := certmagic.New(t.certCache, ap.makeCertMagicConfig(t.ctx))
 	return magic.HandleHTTPChallenge(w, r)
 }
 
 func (t *TLS) getConfigForName(name string) (certmagic.Config, error) {
 	ap := t.getAutomationPolicyForName(name)
-	return ap.makeCertMagicConfig(), nil
+	return ap.makeCertMagicConfig(t.ctx), nil
 }
 
 func (t *TLS) getAutomationPolicyForName(name string) AutomationPolicy {
@@ -178,12 +181,12 @@ type AutomationPolicy struct {
 	management ManagerMaker
 }
 
-func (ap AutomationPolicy) makeCertMagicConfig() certmagic.Config {
+func (ap AutomationPolicy) makeCertMagicConfig(ctx caddy2.Context) certmagic.Config {
 	// default manager (ACME) is a special case because of how CertMagic is designed
 	// TODO: refactor certmagic so that ACME manager is not a special case by extracting
 	// its config fields out of the certmagic.Config struct, or something...
 	if acmeMgmt, ok := ap.management.(*acmeManagerMaker); ok {
-		return acmeMgmt.makeCertMagicConfig()
+		return acmeMgmt.makeCertMagicConfig(ctx)
 	}
 
 	return certmagic.Config{