Skip to content

Commit

Permalink
tpl/partials: Fix partialCached deadlock regression
Browse files Browse the repository at this point in the history
This is a rollback of  0927cf7

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
  • Loading branch information
bep committed Mar 2, 2022
1 parent 376704d commit 9b8b6d3
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 19 deletions.
38 changes: 38 additions & 0 deletions tpl/partials/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
38 changes: 19 additions & 19 deletions tpl/partials/partials.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit 9b8b6d3

Please sign in to comment.