mirror of
https://github.com/gohugoio/hugo.git
synced 2025-12-13 20:36:04 +01:00
hugolib: Fix recently introduced data race
This introduces a new thread safe maps.Map type to avoid abusing the exsting maps.Cache for regular map uses. Fixes #14140
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
// Copyright 2024 The Hugo Authors. All rights reserved.
|
||||
// Copyright 2025 The Hugo Authors. All rights reserved.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
|
||||
117
common/maps/map.go
Normal file
117
common/maps/map.go
Normal file
@@ -0,0 +1,117 @@
|
||||
// Copyright 2025 The Hugo Authors. All rights reserved.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package maps
|
||||
|
||||
import (
|
||||
"iter"
|
||||
"sync"
|
||||
)
|
||||
|
||||
func NewMap[K comparable, T any]() *Map[K, T] {
|
||||
return &Map[K, T]{
|
||||
m: make(map[K]T),
|
||||
}
|
||||
}
|
||||
|
||||
// Map is a thread safe map backed by a Go map.
|
||||
type Map[K comparable, T any] struct {
|
||||
m map[K]T
|
||||
mu sync.RWMutex
|
||||
}
|
||||
|
||||
// Get gets the value for the given key.
|
||||
// It returns the zero value of T if the key is not found.
|
||||
func (m *Map[K, T]) Get(key K) T {
|
||||
v, _ := m.Lookup(key)
|
||||
return v
|
||||
}
|
||||
|
||||
// Lookup looks up the given key in the map.
|
||||
// It returns the value and a boolean indicating whether the key was found.
|
||||
func (m *Map[K, T]) Lookup(key K) (T, bool) {
|
||||
m.mu.RLock()
|
||||
v, found := m.m[key]
|
||||
m.mu.RUnlock()
|
||||
return v, found
|
||||
}
|
||||
|
||||
// GetOrCreate gets the value for the given key if it exists, or creates it if not.
|
||||
func (m *Map[K, T]) GetOrCreate(key K, create func() (T, error)) (T, error) {
|
||||
v, found := m.Lookup(key)
|
||||
if found {
|
||||
return v, nil
|
||||
}
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
v, found = m.m[key]
|
||||
if found {
|
||||
return v, nil
|
||||
}
|
||||
v, err := create()
|
||||
if err != nil {
|
||||
return v, err
|
||||
}
|
||||
m.m[key] = v
|
||||
return v, nil
|
||||
}
|
||||
|
||||
// Set sets the given key to the given value.
|
||||
func (m *Map[K, T]) Set(key K, value T) {
|
||||
m.mu.Lock()
|
||||
m.m[key] = value
|
||||
m.mu.Unlock()
|
||||
}
|
||||
|
||||
// WithWriteLock executes the given function with a write lock on the map.
|
||||
func (m *Map[K, T]) WithWriteLock(f func(m map[K]T)) {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
f(m.m)
|
||||
}
|
||||
|
||||
// SetIfAbsent sets the given key to the given value if the key does not already exist in the map.
|
||||
// It returns true if the value was set, false otherwise.
|
||||
func (m *Map[K, T]) SetIfAbsent(key K, value T) bool {
|
||||
m.mu.RLock()
|
||||
if _, found := m.m[key]; !found {
|
||||
m.mu.RUnlock()
|
||||
return m.doSetIfAbsent(key, value)
|
||||
}
|
||||
m.mu.RUnlock()
|
||||
return false
|
||||
}
|
||||
|
||||
func (m *Map[K, T]) doSetIfAbsent(key K, value T) bool {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
if _, found := m.m[key]; !found {
|
||||
m.m[key] = value
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// All returns an iterator over all key/value pairs in the map.
|
||||
// A read lock is held during the iteration.
|
||||
func (m *Map[K, T]) All() iter.Seq2[K, T] {
|
||||
return func(yield func(K, T) bool) {
|
||||
m.mu.RLock()
|
||||
defer m.mu.RUnlock()
|
||||
for k, v := range m.m {
|
||||
if !yield(k, v) {
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
71
common/maps/map_test.go
Normal file
71
common/maps/map_test.go
Normal file
@@ -0,0 +1,71 @@
|
||||
// Copyright 2025 The Hugo Authors. All rights reserved.
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package maps
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
qt "github.com/frankban/quicktest"
|
||||
)
|
||||
|
||||
func TestMap(t *testing.T) {
|
||||
c := qt.New(t)
|
||||
|
||||
m := NewMap[string, int]()
|
||||
|
||||
m.Set("b", 42)
|
||||
v, found := m.Lookup("b")
|
||||
c.Assert(found, qt.Equals, true)
|
||||
c.Assert(v, qt.Equals, 42)
|
||||
v = m.Get("b")
|
||||
c.Assert(v, qt.Equals, 42)
|
||||
v, found = m.Lookup("c")
|
||||
c.Assert(found, qt.Equals, false)
|
||||
c.Assert(v, qt.Equals, 0)
|
||||
v = m.Get("c")
|
||||
c.Assert(v, qt.Equals, 0)
|
||||
v, err := m.GetOrCreate("d", func() (int, error) {
|
||||
return 100, nil
|
||||
})
|
||||
c.Assert(err, qt.IsNil)
|
||||
c.Assert(v, qt.Equals, 100)
|
||||
v, found = m.Lookup("d")
|
||||
c.Assert(found, qt.Equals, true)
|
||||
c.Assert(v, qt.Equals, 100)
|
||||
|
||||
v, err = m.GetOrCreate("d", func() (int, error) {
|
||||
return 200, nil
|
||||
})
|
||||
c.Assert(err, qt.IsNil)
|
||||
c.Assert(v, qt.Equals, 100)
|
||||
|
||||
wasSet := m.SetIfAbsent("e", 300)
|
||||
c.Assert(wasSet, qt.Equals, true)
|
||||
v, found = m.Lookup("e")
|
||||
c.Assert(found, qt.Equals, true)
|
||||
c.Assert(v, qt.Equals, 300)
|
||||
|
||||
wasSet = m.SetIfAbsent("e", 400)
|
||||
c.Assert(wasSet, qt.Equals, false)
|
||||
v, found = m.Lookup("e")
|
||||
c.Assert(found, qt.Equals, true)
|
||||
c.Assert(v, qt.Equals, 300)
|
||||
|
||||
m.WithWriteLock(func(m map[string]int) {
|
||||
m["f"] = 500
|
||||
})
|
||||
v, found = m.Lookup("f")
|
||||
c.Assert(found, qt.Equals, true)
|
||||
c.Assert(v, qt.Equals, 500)
|
||||
}
|
||||
@@ -48,15 +48,16 @@ type allPagesAssembler struct {
|
||||
r para.Runner
|
||||
|
||||
assembleChanges *WhatChanged
|
||||
seenTerms map[term]sitesmatrix.Vectors
|
||||
droppedPages map[*Site][]string // e.g. drafts, expired, future.
|
||||
seenRootSections *maps.Cache[string, bool]
|
||||
seenHome bool
|
||||
assembleSectionsInParallel bool
|
||||
|
||||
// Internal state.
|
||||
pwRoot *doctree.NodeShiftTreeWalker[contentNode] // walks pages.
|
||||
rwRoot *doctree.NodeShiftTreeWalker[contentNode] // walks resources.
|
||||
|
||||
// Walking state.
|
||||
seenTerms *maps.Map[term, sitesmatrix.Vectors]
|
||||
droppedPages *maps.Map[*Site, []string] // e.g. drafts, expired, future.
|
||||
seenRootSections *maps.Map[string, bool]
|
||||
seenHome bool // set before we fan out to multiple goroutines.
|
||||
}
|
||||
|
||||
func newAllPagesAssembler(
|
||||
@@ -74,7 +75,7 @@ func newAllPagesAssembler(
|
||||
pw := rw.Extend()
|
||||
pw.Tree = m.treePages
|
||||
|
||||
seenRootSections := maps.NewCache[string, bool]()
|
||||
seenRootSections := maps.NewMap[string, bool]()
|
||||
seenRootSections.Set("", true) // home.
|
||||
|
||||
return &allPagesAssembler{
|
||||
@@ -82,8 +83,8 @@ func newAllPagesAssembler(
|
||||
h: h,
|
||||
m: m,
|
||||
assembleChanges: assembleChanges,
|
||||
seenTerms: map[term]sitesmatrix.Vectors{},
|
||||
droppedPages: map[*Site][]string{},
|
||||
seenTerms: maps.NewMap[term, sitesmatrix.Vectors](),
|
||||
droppedPages: maps.NewMap[*Site, []string](),
|
||||
seenRootSections: seenRootSections,
|
||||
assembleSectionsInParallel: true,
|
||||
pwRoot: pw,
|
||||
@@ -100,7 +101,7 @@ type sitePagesAssembler struct {
|
||||
|
||||
func (a *allPagesAssembler) createAllPages() error {
|
||||
defer func() {
|
||||
for site, dropped := range a.droppedPages {
|
||||
for site, dropped := range a.droppedPages.All() {
|
||||
for _, s := range dropped {
|
||||
site.pageMap.treePages.Delete(s)
|
||||
site.pageMap.resourceTrees.DeletePrefix(paths.AddTrailingSlash(s))
|
||||
@@ -112,16 +113,16 @@ func (a *allPagesAssembler) createAllPages() error {
|
||||
defer func() {
|
||||
if a.h.isRebuild() && a.h.previousSeenTerms != nil {
|
||||
// Find removed terms.
|
||||
for t := range a.h.previousSeenTerms {
|
||||
if _, found := a.seenTerms[t]; !found {
|
||||
for t := range a.h.previousSeenTerms.All() {
|
||||
if _, found := a.seenTerms.Lookup(t); !found {
|
||||
// This term has been removed.
|
||||
a.pwRoot.Tree.Delete(t.view.pluralTreeKey)
|
||||
a.rwRoot.Tree.DeletePrefix(t.view.pluralTreeKey + "/")
|
||||
}
|
||||
}
|
||||
// Find new terms.
|
||||
for t := range a.seenTerms {
|
||||
if _, found := a.h.previousSeenTerms[t]; !found {
|
||||
for t := range a.seenTerms.All() {
|
||||
if _, found := a.h.previousSeenTerms.Lookup(t); !found {
|
||||
// This term is new.
|
||||
if n, ok := a.pwRoot.Tree.GetRaw(t.view.pluralTreeKey); ok {
|
||||
a.assembleChanges.Add(cnh.GetIdentity(n))
|
||||
@@ -284,7 +285,12 @@ func (a *allPagesAssembler) doCreatePages(prefix string) error {
|
||||
(&p.m.pageConfig.Build).Disable()
|
||||
default:
|
||||
// Skip this page.
|
||||
a.droppedPages[site] = append(a.droppedPages[site], v.Path())
|
||||
a.droppedPages.WithWriteLock(
|
||||
func(m map[*Site][]string) {
|
||||
m[site] = append(m[site], s)
|
||||
},
|
||||
)
|
||||
|
||||
drop = true
|
||||
}
|
||||
}
|
||||
@@ -471,13 +477,13 @@ func (a *allPagesAssembler) doCreatePages(prefix string) error {
|
||||
}
|
||||
|
||||
if !isResource && s != "" && !a.seenHome {
|
||||
a.seenHome = true
|
||||
var homePages contentNode
|
||||
homePages, ns, err = transformPages("", newHomePageMetaSource(), cascades)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
treePages.InsertRaw("", homePages)
|
||||
a.seenHome = true
|
||||
}
|
||||
|
||||
n2, ns, err = transformPages(s, n, cascades)
|
||||
@@ -633,17 +639,16 @@ func (a *allPagesAssembler) doCreatePages(prefix string) error {
|
||||
continue
|
||||
}
|
||||
t := term{view: viewName, term: v}
|
||||
if vectors, found := a.seenTerms[t]; found {
|
||||
if _, found := vectors[vec]; found {
|
||||
continue
|
||||
a.seenTerms.WithWriteLock(func(m map[term]sitesmatrix.Vectors) {
|
||||
vectors, found := m[t]
|
||||
if !found {
|
||||
m[t] = sitesmatrix.Vectors{
|
||||
vec: struct{}{},
|
||||
}
|
||||
return
|
||||
}
|
||||
vectors[vec] = struct{}{}
|
||||
} else {
|
||||
a.seenTerms[t] = sitesmatrix.Vectors{
|
||||
vec: struct{}{},
|
||||
}
|
||||
}
|
||||
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -752,7 +757,7 @@ func (a *allPagesAssembler) doCreatePages(prefix string) error {
|
||||
// Create missing term pages.
|
||||
pw.WalkContext.HooksPost2().Push(
|
||||
func() error {
|
||||
for k, v := range a.seenTerms {
|
||||
for k, v := range a.seenTerms.All() {
|
||||
viewTermKey := "/" + k.view.plural + "/" + k.term
|
||||
|
||||
pi := a.h.Conf.PathParser().Parse(files.ComponentFolderContent, viewTermKey+"/_index.md")
|
||||
|
||||
@@ -429,9 +429,21 @@ type NodeShiftTreeWalker[T any] struct {
|
||||
// Extend returns a new NodeShiftTreeWalker with the same configuration
|
||||
// and the same WalkContext as the original.
|
||||
// Any local state is reset.
|
||||
func (r NodeShiftTreeWalker[T]) Extend() *NodeShiftTreeWalker[T] {
|
||||
r.skipPrefixes = nil
|
||||
return &r
|
||||
func (r *NodeShiftTreeWalker[T]) Extend() *NodeShiftTreeWalker[T] {
|
||||
return &NodeShiftTreeWalker[T]{
|
||||
Tree: r.Tree,
|
||||
Transform: r.Transform,
|
||||
TransformDelayInsert: r.TransformDelayInsert,
|
||||
Handle: r.Handle,
|
||||
Prefix: r.Prefix,
|
||||
IncludeFilter: r.IncludeFilter,
|
||||
IncludeRawFilter: r.IncludeRawFilter,
|
||||
LockType: r.LockType,
|
||||
NoShift: r.NoShift,
|
||||
Fallback: r.Fallback,
|
||||
Debug: r.Debug,
|
||||
WalkContext: r.WalkContext,
|
||||
}
|
||||
}
|
||||
|
||||
// WithPrefix returns a new NodeShiftTreeWalker with the given prefix.
|
||||
|
||||
@@ -96,8 +96,8 @@ type HugoSites struct {
|
||||
translationKeyPages *maps.SliceCache[page.Page]
|
||||
|
||||
pageTrees *pageTrees
|
||||
previousPageTreesWalkContext *doctree.WalkContext[contentNode] // Set for rebuilds only.
|
||||
previousSeenTerms map[term]sitesmatrix.Vectors // Set for rebuilds only.
|
||||
previousPageTreesWalkContext *doctree.WalkContext[contentNode] // Set for rebuilds only.
|
||||
previousSeenTerms *maps.Map[term, sitesmatrix.Vectors] // Set for rebuilds only.
|
||||
|
||||
printUnusedTemplatesInit sync.Once
|
||||
printPathWarningsInit sync.Once
|
||||
|
||||
Reference in New Issue
Block a user