Repository: caddyserver/caddy (language: go)

Issue:
bbe1952 panic
I use caddy commit: https://github.com/caddyserver/caddy/commit/27bc16abedb0b9afc06705ec839c06d908cd91c9
and
trojan commit: https://github.com/imgk/caddy-trojan/commit/8be66197156d46c0a4f7562f6c9656f4e0c13906
(xcaddy --with github.com/imgk/caddy-trojan@master )
full compile command:
```
CGO_ENABLED=0 GOOS=linux GOARCH=amd64 GOAMD64=v2 xcaddy build 27bc16abedb0b9afc06705ec839c06d908cd91c9 --output /tmp/caddy/caddy --with github.com/mholt/caddy-webdav --with github.com/caddy-dns/cloudflare --with github.com/mholt/caddy-events-exec --with github.com/caddyserver/forwardproxy@caddy2=github.com/klzgrad/forwardproxy@naive --with github.com/WeidiDeng/caddy-cloudflare-ip --with github.com/imgk/caddy-trojan@master --with github.com/mholt/caddy-dynamicdns
```
OS:
```
uname -a
Linux test 6.0.5 #1 SMP PREEMPT_DYNAMIC Thu Oct 27 06:59:14 CST 2022 x86_64 GNU/Linux
```
caddy version:
```
/tmp/caddy/caddy version
v2.7.0-beta.2.0.20230713215448-27bc16abedb0 h1:FZii1qV8q1TfPp6cMK6vPQum5apj29DLptOV5QzbNGI=
```

Simplified caddy configuration file:
```
{
	log {
		output file /tmp/caddy/caddy_log.txt
		level INFO
	}
	trojan {
		caddy
		no_proxy
		users pw1234
	}
	servers 127.0.0.1:88 {
		listener_wrappers {
			trojan
		}
	}
}
:88 {
	bind 127.0.0.1
	route {
		reverse_proxy 127.0.0.1:80
	}
}
```

After caddy starts, it will panic. I don’t know if this is a problem with caddy or a problem with the trojan plugin.
```
/tmp/caddy/caddy run --config /tmp/caddy/Caddyfile
```

log:
```
2023/07/15 02:46:57.148 INFO    using provided configuratio{"config_file": "/tmp/caddy/Caddyfile", "config_adapter": ""}2023/07/15 02:46:57.172 WARN    Caddyfile input is not formatted; run 'caddy fmt --overwrite' to fix inconsistencies   {"adapter": "caddyfile", "file": "/tmp/caddy/Caddyfile", "line": 20}
2023/07/15 02:46:57.182 INFO    redirected default logger  {"from": "stderr", "to": "/tmp/caddy/caddy_log.txt"}
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x10cd562]

goroutine 1 [running]:
github.com/caddyserver/caddy/v2.Context.AppIfConfigured(...)        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/context.go:444
github.com/caddyserver/caddy/v2/modules/caddytls.(*TLS).Cleanup(0xc000671180)
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/modules/caddytls/tls.go:335 +0xa2
github.com/caddyserver/caddy/v2.NewContext.func1()
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/context.go:67 +0x17d
github.com/caddyserver/caddy/v2.run.func1()
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/caddy.go:419 +0x2a
github.com/caddyserver/caddy/v2.run(0xc000464c70?, 0x1)
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/caddy.go:484 +0x81c
github.com/caddyserver/caddy/v2.unsyncedDecodeAndRun({0xc000118780, 0x1c2, 0x1e0}, 0x1)
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/caddy.go:337 +0x145
github.com/caddyserver/caddy/v2.changeConfig({0x1adeb6e, 0x4}, {0x1ae6f91, 0x7}, {0xc0001185a0, 0x1c2, 0x1e0}, {0x0, 0x0}, 0x1)
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/caddy.go:228 +0x79a
github.com/caddyserver/caddy/v2.Load({0xc0001185a0, 0x1c2, 0x1e0}, 0x0?)
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/caddy.go:127 +0x24e
github.com/caddyserver/caddy/v2/cmd.cmdRun({0x0?})
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/cmd/commandfuncs.go:222 +0x945
github.com/caddyserver/caddy/v2/cmd.WrapCommandFuncForCobra.func1(0xc0001dc300?, {0x1adf3b2?, 0x2?, 0x2?})
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/cmd/cobra.go:126 +0x2f
github.com/spf13/cobra.(*Command).execute(0xc0001dc300, {0xc0006551e0, 0x2, 0x2})
        github.com/spf13/cobra@v1.7.0/command.go:940 +0x862
github.com/spf13/cobra.(*Command).ExecuteC(0x2c9a720)
        github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3bdgithub.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/cobra@v1.7.0/command.go:992
github.com/caddyserver/caddy/v2/cmd.Main()
        github.com/caddyserver/caddy/v2@v2.7.0-beta.2.0.20230713215448-27bc16abedb0/cmd/main.go:64 +0x5c
main.main()
        caddy/main.go:18 +0x17
```

After compiling and testing the last few commit versions
Panic started after this commit https://github.com/caddyserver/caddy/commit/0e2c7e1d35b287fc0e56d6db2951f791e09b5a37 



Discussion / hints:
Thanks for opening an issue! And ESPECIALLY thank you for trying the latest commits!! You're awesome. I'll look into this.

It's not immediately clear to me what is going on, so I'll need your help to understand it better.

Ideally, we need to be able to reproduce the bug _in the most minimal way possible_ **using the latest version of Caddy.** This allows us to write regression tests to verify the fix is working. If we can't reproduce it, then you'll have to test our changes for us until it's fixed -- and then we can't add test cases, either.

I've attached a template below that will help make this easier and faster! **This will require some effort on your part -- please understand that we will be dedicating time to fix the bug you are reporting if you can just help us understand it and reproduce it easily.**

This template will ask for some information you've already provided; that's OK, just fill it out the best you can. :+1: I've also included some helpful tips below the template. Feel free to let me know if you have any questions!

Thank you again for your report, we look forward to resolving it!

## Template

<pre><code>## 1. Environment

### 1a. Operating system and version

```
paste here
```


### 1b. Caddy version (run `caddy version` or paste commit SHA)

This should be the latest version of Caddy:

```
paste here
```


## 2. Description

### 2a. What happens (briefly explain what is wrong)




### 2b. Why it's a bug (if it's not obvious)




### 2c. Log output

```
paste terminal output or logs here
```



### 2d. Workaround(s)




### 2e. Relevant links




## 3. Tutorial (minimal steps to reproduce the bug)




</code></pre>

## Instructions -- please heed otherwise we cannot help you (help us help you!)

1. **Environment**: Please fill out your OS and Caddy versions, even if you don't think they are relevant. (They are _always_ relevant.) If you built Caddy from source, provide the commit SHA and specify your exact Go version.

2. **Description:** Describe at a high level what the bug is. What happens? Why is it a bug? Not all bugs are obvious, so convince readers that it's actually a bug.
	- **2c) Log output:** Paste terminal output and/or complete logs in a code block. [DO NOT REDACT INFORMATION except for credentials.](https://caddy.community/t/how-to-get-help-with-caddy-more-effectively/5222) Please enable debug and access logs.
	- **2d) Workaround:** What are you doing to work around the problem in the meantime? This can help others who encounter the same problem, until we implement a fix.
	- **2e) Relevant links:** Please link to any related issues, pull requests, docs, and/or discussion. This can add crucial context to your report.

3. **Tutorial:** What are the _minimum required specific steps_ someone needs to take in order to experience the same bug? Your goal here is to make sure that anyone else can have the same experience with the bug as you do. You are writing a tutorial, so make sure to carry it out yourself before posting it. Please:
	- Start with an empty config. Add _only_ the lines/parameters that are _absolutely required_ to reproduce the bug.
	- Do not run Caddy inside containers.
	- Run Caddy manually in your terminal; do not use systemd or other init systems.
	- If making HTTP requests, avoid web browsers. Use a simpler HTTP client instead, like `curl`.
	- Do not redact any information from your config (except credentials). Domain names are public knowledge and [often necessary for quick resolution of an issue](https://caddy.community/t/how-to-get-help-with-caddy-more-effectively/5222)!
	- Note that ignoring this advice may result in delays, or even in your issue being closed. 😞 Only actionable issues are kept open, and if there is not enough information or clarity to reproduce the bug, then the report is not actionable.

Example of a tutorial:

<blockquote>Create a config file:

```
{ ... }
```

Open terminal and run Caddy:

```
$ caddy ...
```

Make an HTTP request:

```
$ curl ...
```

Notice that the result is ___ but it should be ___.</blockquote>
@text01001 What is your config, at least?
Sorry for the long wait, I re-edited the issue with more details
Can you trigger the problem without the plugin?
> 

 No plug-in will not panic, but caddy commit https://github.com/caddyserver/caddy/commit/7ceef91295343237f5b81ed00e3ba7e8e594d603 The same compilation method and startup will not panic, everything is normal
Thanks @text01001 I can replicate the problem and I understand what's wrong. But fixing it correctly is complicated, I think @mholt will need to find a solution.
Thanks -- dang... of course, my TODO in the code turns out to be prophetic.

I'll work on this this week.
I think I have a fix for this, but am verifying that it works in conjunction with another patch coming up.

Fix patch:
diff --git a/admin.go b/admin.go
index 3ea5051a0a9..4a1d23b60ec 100644
--- a/admin.go
+++ b/admin.go
@@ -1016,9 +1016,9 @@ func handleConfigID(w http.ResponseWriter, r *http.Request) error {
 	id := parts[2]
 
 	// map the ID to the expanded path
-	currentCtxMu.RLock()
+	rawCfgMu.RLock()
 	expanded, ok := rawCfgIndex[id]
-	currentCtxMu.RUnlock()
+	rawCfgMu.RUnlock()
 	if !ok {
 		return APIError{
 			HTTPStatus: http.StatusNotFound,
diff --git a/caddy.go b/caddy.go
index dcaa86b04b7..76d51c833c3 100644
--- a/caddy.go
+++ b/caddy.go
@@ -156,8 +156,8 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
 		return fmt.Errorf("method not allowed")
 	}
 
-	currentCtxMu.Lock()
-	defer currentCtxMu.Unlock()
+	rawCfgMu.Lock()
+	defer rawCfgMu.Unlock()
 
 	if ifMatchHeader != "" {
 		// expect the first and last character to be quotes
@@ -257,8 +257,8 @@ func changeConfig(method, path string, input []byte, ifMatchHeader string, force
 // readConfig traverses the current config to path
 // and writes its JSON encoding to out.
 func readConfig(path string, out io.Writer) error {
-	currentCtxMu.RLock()
-	defer currentCtxMu.RUnlock()
+	rawCfgMu.RLock()
+	defer rawCfgMu.RUnlock()
 	return unsyncedConfigAccess(http.MethodGet, path, nil, out)
 }
 
@@ -305,7 +305,7 @@ func indexConfigObjects(ptr any, configPath string, index map[string]string) err
 // it as the new config, replacing any other current config.
 // It does NOT update the raw config state, as this is a
 // lower-level function; most callers will want to use Load
-// instead. A write lock on currentCtxMu is required! If
+// instead. A write lock on rawCfgMu is required! If
 // allowPersist is false, it will not be persisted to disk,
 // even if it is configured to.
 func unsyncedDecodeAndRun(cfgJSON []byte, allowPersist bool) error {
@@ -340,8 +340,10 @@ func unsyncedDecodeAndRun(cfgJSON []byte, allowPersist bool) error {
 	}
 
 	// swap old context (including its config) with the new one
+	currentCtxMu.Lock()
 	oldCtx := currentCtx
 	currentCtx = ctx
+	currentCtxMu.Unlock()
 
 	// Stop, Cleanup each old app
 	unsyncedStop(oldCtx)
@@ -627,22 +629,35 @@ type ConfigLoader interface {
 // stop the others. Stop should only be called
 // if not replacing with a new config.
 func Stop() error {
+	currentCtxMu.RLock()
+	ctx := currentCtx
+	currentCtxMu.RUnlock()
+
+	rawCfgMu.Lock()
+	unsyncedStop(ctx)
+
 	currentCtxMu.Lock()
-	defer currentCtxMu.Unlock()
-	unsyncedStop(currentCtx)
 	currentCtx = Context{}
+	currentCtxMu.Unlock()
+
 	rawCfgJSON = nil
 	rawCfgIndex = nil
 	rawCfg[rawConfigKey] = nil
+	rawCfgMu.Unlock()
+
 	return nil
 }
 
-// unsyncedStop stops cfg from running, but has
-// no locking around cfg. It is a no-op if cfg is
-// nil. If any app returns an error when stopping,
+// unsyncedStop stops ctx from running, but has
+// no locking around ctx. It is a no-op if ctx has a
+// nil cfg. If any app returns an error when stopping,
 // it is logged and the function continues stopping
 // the next app. This function assumes all apps in
-// cfg were successfully started first.
+// ctx were successfully started first.
+//
+// A lock on rawCfgMu is required, even though this
+// function does not access rawCfg, that lock
+// synchronizes the stop/start of apps.
 func unsyncedStop(ctx Context) {
 	if ctx.cfg == nil {
 		return
@@ -959,9 +974,8 @@ func Version() (simple, full string) {
 // This function is experimental and might be changed
 // or removed in the future.
 func ActiveContext() Context {
-	// TODO: This locking might still be needed; more investigation is required (deadlock during Cleanup for the caddytls.TLS module).
-	// currentCtxMu.RLock()
-	// defer currentCtxMu.RUnlock()
+	currentCtxMu.RLock()
+	defer currentCtxMu.RUnlock()
 	return currentCtx
 }
 
@@ -970,14 +984,12 @@ type CtxKey string
 
 // This group of variables pertains to the current configuration.
 var (
-	// currentCtxMu protects everything in this var block.
-	currentCtxMu sync.RWMutex
-
 	// currentCtx is the root context for the currently-running
 	// configuration, which can be accessed through this value.
 	// If the Config contained in this value is not nil, then
 	// a config is currently active/running.
-	currentCtx Context
+	currentCtx   Context
+	currentCtxMu sync.RWMutex
 
 	// rawCfg is the current, generic-decoded configuration;
 	// we initialize it as a map with one field ("config")
@@ -995,6 +1007,10 @@ var (
 	// rawCfgIndex is the map of user-assigned ID to expanded
 	// path, for converting /id/ paths to /config/ paths.
 	rawCfgIndex map[string]string
+
+	// rawCfgMu protects all the rawCfg fields and also
+	// essentially synchronizes config changes/reloads.
+	rawCfgMu sync.RWMutex
 )
 
 // errSameConfig is returned if the new config is the same
diff --git a/context.go b/context.go
index 004dee60070..85978d49cd8 100644
--- a/context.go
+++ b/context.go
@@ -441,6 +441,12 @@ func (ctx Context) App(name string) (any, error) {
 // or stop App modules. The caller is expected to assert to the
 // concrete type.
 func (ctx Context) AppIfConfigured(name string) any {
+	if ctx.cfg == nil {
+		// this can happen if the currently-active context
+		// is being accessed, but no config has successfully
+		// been loaded yet
+		return nil
+	}
 	return ctx.cfg.apps[name]
 }
 


Test patch: