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

diff --git a/tpl/partials/integration_test.go b/tpl/partials/integration_test.go
index 446e471180964a973d306ced6b2f99ac28f2b032..bda5ddbd5c6374cf1a1ae44e99df13abeb0fd296 100644 (file)
--- 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
index 500f5d1a358cc7f11eda3ea62809b8b2fd7b522a..b18e280ca4b52e048c481097dca37ce7795d6c7d 100644 (file)
--- 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