hugo

commit 9b8b6d34e2d039bfc040fd865a2e77ce2c278587
parent 376704d382df163c7a0db066900f021ea5f7894d
Author: Bjørn Erik Pedersen <bjorn.erik.pedersen@gmail.com>
Date:   Wed,  2 Mar 2022 10:04:29 +0100

tpl/partials: Fix partialCached deadlock regression

This is a rollback of  0927cf739fee9646c7fb917965799d9acf080922

We cannot do that change until we either completes #9570 or possibly also use the new TryLock in GO 1.18.

Fixes #9588
Opens #4086

Diffstat:
M
tpl/partials/integration_test.go
 | 
38
++++++++++++++++++++++++++++++++++++++
M
tpl/partials/partials.go
 | 
38
+++++++++++++++++++-------------------

2 files changed, 57 insertions(+), 19 deletions(-)
diff --git a/tpl/partials/integration_test.go b/tpl/partials/integration_test.go
@@ -103,6 +103,44 @@ P2
 `)
 }
 
+// Issue #588
+func TestIncludeCachedRecursionShortcode(t *testing.T) {
+	t.Parallel()
+
+	files := `
+-- config.toml --
+baseURL = 'http://example.com/'
+-- content/_index.md --
+---
+title: "Index"
+---
+{{< short >}}
+-- layouts/index.html --
+{{ partials.IncludeCached "p1.html" . }}
+-- layouts/partials/p1.html --
+{{ .Content }}
+{{ partials.IncludeCached "p2.html" . }}
+-- layouts/partials/p2.html --
+-- layouts/shortcodes/short.html --
+SHORT
+{{ partials.IncludeCached "p2.html" . }}
+P2
+
+  `
+
+	b := hugolib.NewIntegrationTestBuilder(
+		hugolib.IntegrationTestConfig{
+			T:           t,
+			TxtarString: files,
+		},
+	).Build()
+
+	b.AssertFileContent("public/index.html", `
+SHORT
+P2
+`)
+}
+
 func TestIncludeCacheHints(t *testing.T) {
 	t.Parallel()
 
diff --git a/tpl/partials/partials.go b/tpl/partials/partials.go
@@ -233,21 +233,14 @@ func (ns *Namespace) getOrCreate(ctx context.Context, key partialCacheKey, conte
 		}
 	}()
 
-	// We may already have a write lock.
-	hasLock := tpl.GetHasLockFromContext(ctx)
-
-	if !hasLock {
-		ns.cachedPartials.RLock()
-	}
+	ns.cachedPartials.RLock()
 	p, ok := ns.cachedPartials.p[key]
-	if !hasLock {
-		ns.cachedPartials.RUnlock()
-	}
+	ns.cachedPartials.RUnlock()
 
 	if ok {
 		if ns.deps.Metrics != nil {
 			ns.deps.Metrics.TrackValue(key.templateName(), p, true)
-			// The templates that gets executed is measued in Execute.
+			// The templates that gets executed is measured in Execute.
 			// We need to track the time spent in the cache to
 			// get the totals correct.
 			ns.deps.Metrics.MeasureSince(key.templateName(), start)
@@ -256,21 +249,28 @@ func (ns *Namespace) getOrCreate(ctx context.Context, key partialCacheKey, conte
 		return p, nil
 	}
 
-	if !hasLock {
-		ns.cachedPartials.Lock()
-		defer ns.cachedPartials.Unlock()
-		ctx = tpl.SetHasLockInContext(ctx, true)
-	}
-
-	var name string
-	name, p, err = ns.include(ctx, key.name, context)
+	// This needs to be done outside the lock.
+	// See #9588
+	_, p, err = ns.include(ctx, key.name, context)
 	if err != nil {
 		return nil, err
 	}
 
+	ns.cachedPartials.Lock()
+	defer ns.cachedPartials.Unlock()
+	// Double-check.
+	if p2, ok := ns.cachedPartials.p[key]; ok {
+		if ns.deps.Metrics != nil {
+			ns.deps.Metrics.TrackValue(key.templateName(), p, true)
+			ns.deps.Metrics.MeasureSince(key.templateName(), start)
+		}
+		return p2, nil
+
+	}
 	if ns.deps.Metrics != nil {
-		ns.deps.Metrics.TrackValue(name, p, false)
+		ns.deps.Metrics.TrackValue(key.templateName(), p, false)
 	}
+
 	ns.cachedPartials.p[key] = p
 
 	return p, nil