diff options
author | Bjørn Erik Pedersen <[email protected]> | 2022-03-02 10:04:29 +0100 |
---|---|---|
committer | Bjørn Erik Pedersen <[email protected]> | 2022-03-02 11:16:21 +0100 |
commit | 9b8b6d34e2d039bfc040fd865a2e77ce2c278587 (patch) | |
tree | c8324f237e7de24a50f83034e254f44d50bd20dc /tpl | |
parent | 376704d382df163c7a0db066900f021ea5f7894d (diff) | |
download | hugo-9b8b6d34e2d039bfc040fd865a2e77ce2c278587.tar.gz hugo-9b8b6d34e2d039bfc040fd865a2e77ce2c278587.zip |
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 (limited to 'tpl')
-rw-r--r-- | tpl/partials/integration_test.go | 38 | ||||
-rw-r--r-- | 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 index 446e47118..bda5ddbd5 100644 --- 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 500f5d1a3..b18e280ca 100644 --- 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 |