From 996875820142dea9c4065cac941cf22118d8c946 Mon Sep 17 00:00:00 2001
From: James Ko <james.llewellyn.ko@gmail.com>
Date: Wed, 2 Sep 2026 17:33:25 -0400
Subject: [PATCH] usagepool: avoid lock inversion after constructor failure
 (#7968)

---
 usagepool.go      | 14 +++++------
 usagepool_test.go | 63 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+), 7 deletions(-)
 create mode 100644 usagepool_test.go

diff --git a/usagepool.go b/usagepool.go
index 6b7a3c25e..13725d31e 100644
--- a/usagepool.go
+++ b/usagepool.go
@@ -94,18 +94,18 @@ func (up *UsagePool) LoadOrNew(key any, construct Constructor) (value any, loade
 		value, err = construct()
 		if err == nil {
 			upv.value = value
+			upv.Unlock()
 		} else {
 			upv.err = err
+			upv.Unlock()
 			up.Lock()
-			// this *should* be safe, I think, because we have a
-			// write lock on upv, but we might also need to ensure
-			// that upv.err is nil before doing this, since we
-			// released the write lock on up during construct...
-			// but then again it's also after midnight...
-			delete(up.pool, key)
+			// Another constructor may have replaced a failed entry while
+			// this goroutine waited for the pool lock.
+			if up.pool[key] == upv {
+				delete(up.pool, key)
+			}
 			up.Unlock()
 		}
-		upv.Unlock()
 	}
 	return value, loaded, err
 }
diff --git a/usagepool_test.go b/usagepool_test.go
new file mode 100644
index 000000000..5a51963bd
--- /dev/null
+++ b/usagepool_test.go
@@ -0,0 +1,63 @@
+// Copyright 2015 Matthew Holt and The Caddy Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package caddy
+
+import (
+	"errors"
+	"testing"
+	"time"
+)
+
+func TestUsagePoolFailedConstructorDoesNotInvertLocks(t *testing.T) {
+	pool := NewUsagePool()
+	constructorStarted := make(chan struct{})
+	finishConstructor := make(chan struct{})
+	loadResult := make(chan error, 1)
+	constructErr := errors.New("construction failed")
+
+	go func() {
+		_, _, err := pool.LoadOrNew("key", func() (Destructor, error) {
+			close(constructorStarted)
+			<-finishConstructor
+			return nil, constructErr
+		})
+		loadResult <- err
+	}()
+
+	<-constructorStarted
+	pool.RLock()
+	entry := pool.pool["key"]
+	close(finishConstructor)
+
+	entryReadable := make(chan struct{})
+	go func() {
+		entry.RLock()
+		entry.RUnlock()
+		close(entryReadable)
+	}()
+
+	select {
+	case <-entryReadable:
+		pool.RUnlock()
+	case <-time.After(time.Second):
+		pool.RUnlock()
+		<-loadResult
+		t.Fatal("failed constructor held the entry lock while waiting for the pool lock")
+	}
+
+	if err := <-loadResult; !errors.Is(err, constructErr) {
+		t.Fatalf("LoadOrNew() error = %v, want %v", err, constructErr)
+	}
+}