@marcel91200 / caddy Mirror · read-only / commits / b51dc5d5
Fork 0

Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS

Mirrored from https://github.com/Slazee/caddy.git

Synced Nov 13, 00:30 UTC

core: Refine mutex during reloads (fix #5628) (#5645)

Separate currentCtxMu to protect currentCtx, and a new rawCfgMu to protect rawCfg and synchronize loads.

Matt Holt committed Jul 21, 2023 at 15:32 UTC b51dc5d5d0b8764165170af1f54b77d6de8cb5a1
3 files changed +42 -20
admin.go
+2 -2
@@ -1016,9 +1016,9 @@ func handleConfigID(w http.ResponseWriter, r *http.Request) error {
1016 id := parts[2]
1017
1018 // map the ID to the expanded path
1019 - currentCtxMu.RLock()
1019 + rawCfgMu.RLock()
1020 expanded, ok := rawCfgIndex[id]
1021 - currentCtxMu.RUnlock()
1021 + rawCfgMu.RUnlock()
1022 if !ok {
1023 return APIError{
1024 HTTPStatus: http.StatusNotFound,
caddy.go
+34 -18
@@ -156,8 +156,8 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
156 return fmt.Errorf("method not allowed")
157 }
158
159 - currentCtxMu.Lock()
160 - defer currentCtxMu.Unlock()
159 + rawCfgMu.Lock()
160 + defer rawCfgMu.Unlock()
161
162 if ifMatchHeader != "" {
163 // expect the first and last character to be quotes
@@ -257,8 +257,8 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
257 // readConfig traverses the current config to path
258 // and writes its JSON encoding to out.
259 func readConfig(path string, out io.Writer) error {
260 - currentCtxMu.RLock()
261 - defer currentCtxMu.RUnlock()
260 + rawCfgMu.RLock()
261 + defer rawCfgMu.RUnlock()
262 return unsyncedConfigAccess(http.MethodGet, path, nil, out)
263 }
264
@@ -305,7 +305,7 @@ func indexConfigObjects(ptr any, configPath string, index map[string]string) err
305 // it as the new config, replacing any other current config.
306 // It does NOT update the raw config state, as this is a
307 // lower-level function; most callers will want to use Load
308 -// instead. A write lock on currentCtxMu is required! If
308 +// instead. A write lock on rawCfgMu is required! If
309 // allowPersist is false, it will not be persisted to disk,
310 // even if it is configured to.
311 func unsyncedDecodeAndRun(cfgJSON []byte, allowPersist bool) error {
@@ -340,8 +340,10 @@ func unsyncedDecodeAndRun(cfgJSON []byte, allowPersist bool) error {
340 }
341
342 // swap old context (including its config) with the new one
343 + currentCtxMu.Lock()
344 oldCtx := currentCtx
345 currentCtx = ctx
346 + currentCtxMu.Unlock()
347
348 // Stop, Cleanup each old app
349 unsyncedStop(oldCtx)
@@ -627,22 +629,35 @@ type ConfigLoader interface {
629 // stop the others. Stop should only be called
630 // if not replacing with a new config.
631 func Stop() error {
632 + currentCtxMu.RLock()
633 + ctx := currentCtx
634 + currentCtxMu.RUnlock()
635 +
636 + rawCfgMu.Lock()
637 + unsyncedStop(ctx)
638 +
639 currentCtxMu.Lock()
631 - defer currentCtxMu.Unlock()
632 - unsyncedStop(currentCtx)
640 currentCtx = Context{}
641 + currentCtxMu.Unlock()
642 +
643 rawCfgJSON = nil
644 rawCfgIndex = nil
645 rawCfg[rawConfigKey] = nil
646 + rawCfgMu.Unlock()
647 +
648 return nil
649 }
650
640 -// unsyncedStop stops cfg from running, but has
641 -// no locking around cfg. It is a no-op if cfg is
642 -// nil. If any app returns an error when stopping,
651 +// unsyncedStop stops ctx from running, but has
652 +// no locking around ctx. It is a no-op if ctx has a
653 +// nil cfg. If any app returns an error when stopping,
654 // it is logged and the function continues stopping
655 // the next app. This function assumes all apps in
645 -// cfg were successfully started first.
656 +// ctx were successfully started first.
657 +//
658 +// A lock on rawCfgMu is required, even though this
659 +// function does not access rawCfg, that lock
660 +// synchronizes the stop/start of apps.
661 func unsyncedStop(ctx Context) {
662 if ctx.cfg == nil {
663 return
@@ -959,9 +974,8 @@ func Version() (simple, full string) {
974 // This function is experimental and might be changed
975 // or removed in the future.
976 func ActiveContext() Context {
962 - // TODO: This locking might still be needed; more investigation is required (deadlock during Cleanup for the caddytls.TLS module).
963 - // currentCtxMu.RLock()
964 - // defer currentCtxMu.RUnlock()
977 + currentCtxMu.RLock()
978 + defer currentCtxMu.RUnlock()
979 return currentCtx
980 }
981
@@ -970,14 +984,12 @@ type CtxKey string
984
985 // This group of variables pertains to the current configuration.
986 var (
973 - // currentCtxMu protects everything in this var block.
974 - currentCtxMu sync.RWMutex
975 -
987 // currentCtx is the root context for the currently-running
988 // configuration, which can be accessed through this value.
989 // If the Config contained in this value is not nil, then
990 // a config is currently active/running.
980 - currentCtx Context
991 + currentCtx Context
992 + currentCtxMu sync.RWMutex
993
994 // rawCfg is the current, generic-decoded configuration;
995 // we initialize it as a map with one field ("config")
@@ -995,6 +1007,10 @@ var (
1007 // rawCfgIndex is the map of user-assigned ID to expanded
1008 // path, for converting /id/ paths to /config/ paths.
1009 rawCfgIndex map[string]string
1010 +
1011 + // rawCfgMu protects all the rawCfg fields and also
1012 + // essentially synchronizes config changes/reloads.
1013 + rawCfgMu sync.RWMutex
1014 )
1015
1016 // errSameConfig is returned if the new config is the same
context.go
+6
@@ -441,6 +441,12 @@ func (ctx Context) App(name string) (any, error) {
441 // or stop App modules. The caller is expected to assert to the
442 // concrete type.
443 func (ctx Context) AppIfConfigured(name string) any {
444 + if ctx.cfg == nil {
445 + // this can happen if the currently-active context
446 + // is being accessed, but no config has successfully
447 + // been loaded yet
448 + return nil
449 + }
450 return ctx.cfg.apps[name]
451 }
452