diff options
author | Bjørn Erik Pedersen <[email protected]> | 2024-11-06 10:17:34 +0100 |
---|---|---|
committer | Bjørn Erik Pedersen <[email protected]> | 2024-11-06 12:17:30 +0100 |
commit | 95e2d5beb8cc5937792e1ed15589434987590e39 (patch) | |
tree | 5d351ce750742b4780ba004d6a4e64893e890efe /hugolib | |
parent | 2c3efc81064a6e0bdde3d02629f06ca87a7d2c08 (diff) | |
download | hugo-95e2d5beb8cc5937792e1ed15589434987590e39.tar.gz hugo-95e2d5beb8cc5937792e1ed15589434987590e39.zip |
Fix concurrent map read and map write in short page lookups
Regression introduced in Hugo `v0.137.0`.
Fixes #13019
Diffstat (limited to 'hugolib')
-rw-r--r-- | hugolib/content_map_page.go | 76 | ||||
-rw-r--r-- | hugolib/content_map_test.go | 76 |
2 files changed, 113 insertions, 39 deletions
diff --git a/hugolib/content_map_page.go b/hugolib/content_map_page.go index 5e8646b21..8c9e4a31a 100644 --- a/hugolib/content_map_page.go +++ b/hugolib/content_map_page.go @@ -37,7 +37,6 @@ import ( "github.com/gohugoio/hugo/hugolib/doctree" "github.com/gohugoio/hugo/hugolib/pagesfromdata" "github.com/gohugoio/hugo/identity" - "github.com/gohugoio/hugo/lazy" "github.com/gohugoio/hugo/media" "github.com/gohugoio/hugo/output" "github.com/gohugoio/hugo/resources" @@ -925,59 +924,58 @@ func newPageMap(i int, s *Site, mcache *dynacache.Cache, pageTrees *pageTrees) * s: s, } - m.pageReverseIndex = &contentTreeReverseIndex{ - initFn: func(rm map[any]contentNodeI) { - add := func(k string, n contentNodeI) { - existing, found := rm[k] - if found && existing != ambiguousContentNode { - rm[k] = ambiguousContentNode - } else if !found { - rm[k] = n - } + m.pageReverseIndex = newContentTreeTreverseIndex(func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI)) { + add := func(k string, n contentNodeI) { + existing, found := get(k) + if found && existing != ambiguousContentNode { + set(k, ambiguousContentNode) + } else if !found { + set(k, n) } + } - w := &doctree.NodeShiftTreeWalker[contentNodeI]{ - Tree: m.treePages, - LockType: doctree.LockTypeRead, - Handle: func(s string, n contentNodeI, match doctree.DimensionFlag) (bool, error) { - p := n.(*pageState) - if p.PathInfo() != nil { - add(p.PathInfo().BaseNameNoIdentifier(), p) - } - return false, nil - }, - } + w := &doctree.NodeShiftTreeWalker[contentNodeI]{ + Tree: m.treePages, + LockType: doctree.LockTypeRead, + Handle: func(s string, n contentNodeI, match doctree.DimensionFlag) (bool, error) { + p := n.(*pageState) + if p.PathInfo() != nil { + add(p.PathInfo().BaseNameNoIdentifier(), p) + } + return false, nil + }, + } - if err := w.Walk(context.Background()); err != nil { - panic(err) - } - }, - contentTreeReverseIndexMap: &contentTreeReverseIndexMap{}, - } + if err := w.Walk(context.Background()); err != nil { + panic(err) + } + }) return m } +func newContentTreeTreverseIndex(init func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI))) *contentTreeReverseIndex { + return &contentTreeReverseIndex{ + initFn: init, + mm: maps.NewCache[any, contentNodeI](), + } +} + type contentTreeReverseIndex struct { - initFn func(rm map[any]contentNodeI) - *contentTreeReverseIndexMap + initFn func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI)) + mm *maps.Cache[any, contentNodeI] } func (c *contentTreeReverseIndex) Reset() { - c.init.ResetWithLock().Unlock() + c.mm.Reset() } func (c *contentTreeReverseIndex) Get(key any) contentNodeI { - c.init.Do(func() { - c.m = make(map[any]contentNodeI) - c.initFn(c.contentTreeReverseIndexMap.m) + v, _ := c.mm.InitAndGet(key, func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI)) error { + c.initFn(get, set) + return nil }) - return c.m[key] -} - -type contentTreeReverseIndexMap struct { - init lazy.OnceMore - m map[any]contentNodeI + return v } type sitePagesAssembler struct { diff --git a/hugolib/content_map_test.go b/hugolib/content_map_test.go index bf9920071..a9f719f4a 100644 --- a/hugolib/content_map_test.go +++ b/hugolib/content_map_test.go @@ -17,9 +17,11 @@ import ( "fmt" "path/filepath" "strings" + "sync" "testing" qt "github.com/frankban/quicktest" + "github.com/gohugoio/hugo/identity" ) func TestContentMapSite(t *testing.T) { @@ -396,3 +398,77 @@ irrelevant "<loc>https://example.org/en/sitemap.xml</loc>", ) } + +func TestContentTreeReverseIndex(t *testing.T) { + t.Parallel() + + c := qt.New(t) + + pageReverseIndex := newContentTreeTreverseIndex( + func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI)) { + for i := 0; i < 10; i++ { + key := fmt.Sprint(i) + set(key, &testContentNode{key: key}) + } + }, + ) + + for i := 0; i < 10; i++ { + key := fmt.Sprint(i) + v := pageReverseIndex.Get(key) + c.Assert(v, qt.Not(qt.IsNil)) + c.Assert(v.Path(), qt.Equals, key) + } +} + +// Issue 13019. +func TestContentTreeReverseIndexPara(t *testing.T) { + t.Parallel() + + var wg sync.WaitGroup + + for i := 0; i < 10; i++ { + pageReverseIndex := newContentTreeTreverseIndex( + func(get func(key any) (contentNodeI, bool), set func(key any, val contentNodeI)) { + for i := 0; i < 10; i++ { + key := fmt.Sprint(i) + set(key, &testContentNode{key: key}) + } + }, + ) + + for j := 0; j < 10; j++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + pageReverseIndex.Get(fmt.Sprint(i)) + }(j) + } + } +} + +type testContentNode struct { + key string +} + +func (n *testContentNode) GetIdentity() identity.Identity { + return identity.StringIdentity(n.key) +} + +func (n *testContentNode) ForEeachIdentity(cb func(id identity.Identity) bool) bool { + panic("not supported") +} + +func (n *testContentNode) Path() string { + return n.key +} + +func (n *testContentNode) isContentNodeBranch() bool { + return false +} + +func (n *testContentNode) resetBuildState() { +} + +func (n *testContentNode) MarkStale() { +} |