dynacache: Fix potential deadlocks on panics in GetOrCreate

[ci skip]

cache/dynacache/dynacache_test.go

@@ -14,8 +14,10 @@
 package dynacache
 
 import (
+	"fmt"
 	"path/filepath"
 	"testing"
+	"time"
 
 	qt "github.com/frankban/quicktest"
 	"github.com/gohugoio/hugo/common/loggers"
@@ -165,6 +167,29 @@ func TestClear(t *testing.T) {
 	cache.adjustCurrentMaxSize()
 }
 
+func TestPanicInCreate(t *testing.T) {
+	t.Parallel()
+	c := qt.New(t)
+	cache := newTestCache(t)
+
+	p1 := GetOrCreatePartition[string, testItem](cache, "/aaaa/bbbb", OptionsPartition{Weight: 30, ClearWhen: ClearOnRebuild})
+
+	for i := 0; i < 3; i++ {
+		for j := 0; j < 3; j++ {
+			_, err := p1.GetOrCreate(fmt.Sprintf("panic1-%d", i), func(string) (testItem, error) {
+				panic("failed")
+			})
+
+			c.Assert(err, qt.Not(qt.IsNil))
+
+			_, err = p1.GetOrCreateWitTimeout(fmt.Sprintf("panic2-%d", i), 10*time.Second, func(string) (testItem, error) {
+				panic("failed")
+			})
+			c.Assert(err, qt.Not(qt.IsNil))
+		}
+	}
+}
+
 func TestAdjustCurrentMaxSize(t *testing.T) {
 	t.Parallel()
 	c := qt.New(t)

commands/commandeer.go

@@ -507,7 +507,7 @@ func (r *rootCommand) createLogger(running bool) (loggers.Logger, error) {
 	return loggers.New(optsLogger), nil
 }
 
-func (r *rootCommand) Reset() {
+func (r *rootCommand) resetLogs() {
 	r.logger.Reset()
 	loggers.Log().Reset()
 }

commands/hugobuilder.go

@@ -27,7 +27,6 @@ import (
 	"sync/atomic"
 	"time"
 
-	"github.com/bep/logg"
 	"github.com/bep/simplecobra"
 	"github.com/fsnotify/fsnotify"
 	"github.com/gohugoio/hugo/common/herrors"
@@ -136,10 +135,6 @@ func (e *hugoBuilderErrState) wasErr() bool {
 	return e.waserr
 }
 
-func (c *hugoBuilder) errCount() int {
-	return c.r.logger.LoggCount(logg.LevelError) + loggers.Log().LoggCount(logg.LevelError)
-}
-
 // getDirList provides NewWatcher() with a list of directories to watch for changes.
 func (c *hugoBuilder) getDirList() ([]string, error) {
 	h, err := c.hugo()
@@ -345,7 +340,6 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
 		for {
 			select {
 			case changes := <-c.r.changesFromBuild:
-				c.errState.setBuildErr(nil)
 				unlock, err := h.LockBuild()
 				if err != nil {
 					c.r.logger.Errorln("Failed to acquire a build lock: %s", err)
@@ -358,7 +352,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
 				}
 				if c.s != nil && c.s.doLiveReload {
 					doReload := c.changeDetector == nil || len(c.changeDetector.changed()) > 0
-					doReload = doReload || c.showErrorInBrowser && c.errCount() > 0
+					doReload = doReload || c.showErrorInBrowser && c.errState.buildErr() != nil
 					if doReload {
 						livereload.ForceRefresh()
 					}
@@ -372,7 +366,7 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
 					return
 				}
 				c.handleEvents(watcher, staticSyncer, evs, configSet)
-				if c.showErrorInBrowser && c.errCount() > 0 {
+				if c.showErrorInBrowser && c.errState.buildErr() != nil {
 					// Need to reload browser to show the error
 					livereload.ForceRefresh()
 				}
@@ -419,11 +413,17 @@ func (c *hugoBuilder) build() error {
 }
 
 func (c *hugoBuilder) buildSites(noBuildLock bool) (err error) {
-	h, err := c.hugo()
+	defer func() {
+		c.errState.setBuildErr(err)
+	}()
+
+	var h *hugolib.HugoSites
+	h, err = c.hugo()
 	if err != nil {
-		return err
+		return
 	}
-	return h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
+	err = h.Build(hugolib.BuildCfg{NoBuildLock: noBuildLock})
+	return
 }
 
 func (c *hugoBuilder) copyStatic() (map[string]uint64, error) {
@@ -619,6 +619,9 @@ func (c *hugoBuilder) fullRebuild(changeType string) {
 			// Set the processing on pause until the state is recovered.
 			c.errState.setPaused(true)
 			c.handleBuildErr(err, "Failed to reload config")
+			if c.s.doLiveReload {
+				livereload.ForceRefresh()
+			}
 		} else {
 			c.errState.setPaused(false)
 		}
@@ -1081,37 +1084,44 @@ func (c *hugoBuilder) printChangeDetected(typ string) {
 	c.r.logger.Println(htime.Now().Format(layout))
 }
 
-func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
+func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) (err error) {
+	defer func() {
+		c.errState.setBuildErr(err)
+	}()
 	if err := c.errState.buildErr(); err != nil {
 		ferrs := herrors.UnwrapFileErrorsWithErrorContext(err)
 		for _, err := range ferrs {
 			events = append(events, fsnotify.Event{Name: err.Position().Filename, Op: fsnotify.Write})
 		}
 	}
-	c.errState.setBuildErr(nil)
-	h, err := c.hugo()
+	var h *hugolib.HugoSites
+	h, err = c.hugo()
 	if err != nil {
-		return err
+		return
 	}
-
-	return h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
+	err = h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()}, events...)
+	return
 }
 
-func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) error {
-	c.errState.setBuildErr(nil)
-	h, err := c.hugo()
+func (c *hugoBuilder) rebuildSitesForChanges(ids []identity.Identity) (err error) {
+	defer func() {
+		c.errState.setBuildErr(err)
+	}()
+
+	var h *hugolib.HugoSites
+	h, err = c.hugo()
 	if err != nil {
-		return err
+		return
 	}
 	whatChanged := &hugolib.WhatChanged{}
 	whatChanged.Add(ids...)
 	err = h.Build(hugolib.BuildCfg{NoBuildLock: true, WhatChanged: whatChanged, RecentlyVisited: c.visitedURLs, ErrRecovery: c.errState.wasErr()})
-	c.errState.setBuildErr(err)
-	return err
+
+	return
 }
 
 func (c *hugoBuilder) reloadConfig() error {
-	c.r.Reset()
+	c.r.resetLogs()
 	c.r.configVersionID.Add(1)
 
 	if err := c.withConfE(func(conf *commonConfig) error {

commands/server.go

@@ -648,9 +648,8 @@ func (c *serverCommand) setServerInfoInConfig() error {
 }
 
 func (c *serverCommand) getErrorWithContext() any {
-	errCount := c.errCount()
-
-	if errCount == 0 {
+	buildErr := c.errState.buildErr()
+	if buildErr == nil {
 		return nil
 	}
 
@@ -659,7 +658,7 @@ func (c *serverCommand) getErrorWithContext() any {
 	m["Error"] = cleanErrorLog(c.r.logger.Errors())
 
 	m["Version"] = hugo.BuildVersionString()
-	ferrors := herrors.UnwrapFileErrorsWithErrorContext(c.errState.buildErr())
+	ferrors := herrors.UnwrapFileErrorsWithErrorContext(buildErr)
 	m["Files"] = ferrors
 
 	return m
@@ -830,22 +829,25 @@ func (c *serverCommand) fixURL(baseURLFromConfig, baseURLFromFlag string, port i
 	return u.String(), nil
 }
 
-func (c *serverCommand) partialReRender(urls ...string) error {
+func (c *serverCommand) partialReRender(urls ...string) (err error) {
 	defer func() {
 		c.errState.setWasErr(false)
 	}()
-	c.errState.setBuildErr(nil)
 	visited := types.NewEvictingStringQueue(len(urls))
 	for _, url := range urls {
 		visited.Add(url)
 	}
 
-	h, err := c.hugo()
+	var h *hugolib.HugoSites
+	h, err = c.hugo()
 	if err != nil {
-		return err
+		return
 	}
+
 	// Note: We do not set NoBuildLock as the file lock is not acquired at this stage.
-	return h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})
+	err = h.Build(hugolib.BuildCfg{NoBuildLock: false, RecentlyVisited: visited, PartialReRender: true, ErrRecovery: c.errState.wasErr()})
+
+	return
 }
 
 func (c *serverCommand) serve() error {

go.mod

@@ -16,7 +16,7 @@ require (
 	github.com/bep/gowebp v0.3.0
 	github.com/bep/helpers v0.5.0
 	github.com/bep/imagemeta v0.8.1
-	github.com/bep/lazycache v0.4.0
+	github.com/bep/lazycache v0.6.0
 	github.com/bep/logg v0.4.0
 	github.com/bep/mclib v1.20400.20402
 	github.com/bep/overlayfs v0.9.2

go.sum

@@ -143,6 +143,8 @@ github.com/bep/imagemeta v0.8.1 h1:tjZLPRftjxU7PTI87o5e5WKOFQ4S9S0engiP1OTpJTI=
 github.com/bep/imagemeta v0.8.1/go.mod h1:5piPAq5Qomh07m/dPPCLN3mDJyFusvUG7VwdRD/vX0s=
 github.com/bep/lazycache v0.4.0 h1:X8yVyWNVupPd4e1jV7efi3zb7ZV/qcjKQgIQ5aPbkYI=
 github.com/bep/lazycache v0.4.0/go.mod h1:NmRm7Dexh3pmR1EignYR8PjO2cWybFQ68+QgY3VMCSc=
+github.com/bep/lazycache v0.6.0 h1:0vCgFo7TBtMQpSx64jnH1sagmw0ZougIFRpsqPHTa5U=
+github.com/bep/lazycache v0.6.0/go.mod h1:NmRm7Dexh3pmR1EignYR8PjO2cWybFQ68+QgY3VMCSc=
 github.com/bep/logg v0.4.0 h1:luAo5mO4ZkhA5M1iDVDqDqnBBnlHjmtZF6VAyTp+nCQ=
 github.com/bep/logg v0.4.0/go.mod h1:Ccp9yP3wbR1mm++Kpxet91hAZBEQgmWgFgnXX3GkIV0=
 github.com/bep/mclib v1.20400.20402 h1:olpCE2WSPpOAbFE1R4hnftSEmQ34+xzy2HRzd0m69rA=

hugolib/hugo_sites.go

@@ -179,9 +179,6 @@ type hugoSitesInit struct {
 	// Loads the data from all of the /data folders.
 	data *lazy.Init
 
-	// Performs late initialization (before render) of the templates.
-	layouts *lazy.Init
-
 	// Loads the Git info and CODEOWNERS for all the pages if enabled.
 	gitInfo *lazy.Init
 }

hugolib/hugo_sites_build.go

@@ -250,10 +250,6 @@ func (h *HugoSites) process(ctx context.Context, l logg.LevelLogger, config *Bui
 	l = l.WithField("step", "process")
 	defer loggers.TimeTrackf(l, time.Now(), nil, "")
 
-	if _, err := h.init.layouts.Do(ctx); err != nil {
-		return err
-	}
-
 	if len(events) > 0 {
 		// This is a rebuild triggered from file events.
 		return h.processPartialFileEvents(ctx, l, config, init, events)
@@ -1067,8 +1063,6 @@ func (h *HugoSites) processPartialFileEvents(ctx context.Context, l logg.LevelLo
 	}
 
 	if tmplChanged || i18nChanged {
-		// TODO(bep) we should split this, but currently the loading of i18n and layout files are tied together. See #12048.
-		h.init.layouts.Reset()
 		if err := loggers.TimeTrackfn(func() (logg.LevelLogger, error) {
 			// TODO(bep) this could probably be optimized to somehow
 			// only load the changed templates and its dependencies, but that is non-trivial.
@@ -1141,10 +1135,6 @@ func (s *Site) handleContentAdapterChanges(bi pagesfromdata.BuildInfo, buildConf
 }
 
 func (h *HugoSites) processContentAdaptersOnRebuild(ctx context.Context, buildConfig *BuildCfg) error {
-	// Make sure the layouts are initialized.
-	if _, err := h.init.layouts.Do(context.Background()); err != nil {
-		return err
-	}
 	g := rungroup.Run[*pagesfromdata.PagesFromTemplate](ctx, rungroup.Config[*pagesfromdata.PagesFromTemplate]{
 		NumWorkers: h.numWorkers,
 		Handle: func(ctx context.Context, p *pagesfromdata.PagesFromTemplate) error {

hugolib/integrationtest_builder.go

@@ -246,11 +246,6 @@ func (s *IntegrationTestBuilder) AssertBuildCountGitInfo(count int) {
 	s.Assert(s.H.init.gitInfo.InitCount(), qt.Equals, count)
 }
 
-func (s *IntegrationTestBuilder) AssertBuildCountLayouts(count int) {
-	s.Helper()
-	s.Assert(s.H.init.layouts.InitCount(), qt.Equals, count)
-}
-
 func (s *IntegrationTestBuilder) AssertFileCount(dirname string, expected int) {
 	s.Helper()
 	fs := s.fs.WorkingDirReadOnly

hugolib/page__new.go

@@ -34,6 +34,15 @@ import (
 var pageIDCounter atomic.Uint64
 
 func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) {
+	p, pth, err := h.doNewPage(m)
+	if err != nil {
+		// Make sure that any partially created page part is marked as stale.
+		m.MarkStale()
+	}
+	return p, pth, err
+}
+
+func (h *HugoSites) doNewPage(m *pageMeta) (*pageState, *paths.Path, error) {
 	m.Staler = &resources.AtomicStaler{}
 	if m.pageMetaParams == nil {
 		m.pageMetaParams = &pageMetaParams{
@@ -231,10 +240,6 @@ func (h *HugoSites) newPage(m *pageMeta) (*pageState, *paths.Path, error) {
 		}
 		return ps, nil
 	}()
-	// Make sure to evict any cached and now stale data.
-	if err != nil {
-		m.MarkStale()
-	}
 
 	if ps == nil {
 		return nil, nil, err

hugolib/site.go

@@ -344,7 +344,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites []
 		skipRebuildForFilenames: make(map[string]bool),
 		init: &hugoSitesInit{
 			data:    lazy.New(),
-			layouts: lazy.New(),
 			gitInfo: lazy.New(),
 		},
 	}
@@ -400,15 +399,6 @@ func newHugoSites(cfg deps.DepsCfg, d *deps.Deps, pageTrees *pageTrees, sites []
 		return nil, nil
 	})
 
-	h.init.layouts.Add(func(context.Context) (any, error) {
-		for _, s := range h.Sites {
-			if err := s.Tmpl().(tpl.TemplateManager).MarkReady(); err != nil {
-				return nil, err
-			}
-		}
-		return nil, nil
-	})
-
 	h.init.gitInfo.Add(func(context.Context) (any, error) {
 		err := h.loadGitInfo()
 		if err != nil {

hugoreleaser.env

@@ -1,7 +1,8 @@
 # Release env. 
 # These will be replaced by script before release.
-HUGORELEASER_TAG=v0.136.3
-HUGORELEASER_COMMITISH=bfa2fd683e7f0874c7f7e2198ec407a037dadf14
+HUGORELEASER_TAG=v0.136.5
+HUGORELEASER_COMMITISH=46cccb021bc6425455f4eec093f5cc4a32f1d12c
+
 
 
 

testscripts/commands/server__error_recovery_edit_config.txt

@@ -0,0 +1,42 @@
+# Test the hugo server command when adding an error to a config file
+# and then fixing it.
+
+hugo server &
+
+waitServer
+
+httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'
+
+replace $WORK/hugo.toml 'title =' 'titlefoo'
+httpget ${HUGOTEST_BASEURL_0}p1/ 'failed'
+
+replace $WORK/hugo.toml 'titlefoo' 'title ='
+httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'
+
+stopServer
+
+-- hugo.toml --
+title = "Hugo Server Test"
+baseURL = "https://example.org/"
+disableKinds = ["taxonomy", "term", "sitemap"]
+-- layouts/index.html --
+Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
+-- layouts/_default/single.html --
+Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
+-- content/_index.md --
+---
+title: Hugo Home
+---
+-- content/p1/index.md --
+---
+title: P1
+---
+-- content/p2/index.md --
+---
+title: P2
+---
+-- static/staticfiles/static.txt --
+static
+
+
+

testscripts/commands/server__error_recovery_edit_content.txt

@@ -0,0 +1,42 @@
+# Test the hugo server command when adding a front matter error to a content file
+# and then fixing it.
+
+hugo server &
+
+waitServer
+
+httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'
+
+replace $WORK/content/p1/index.md 'title:' 'titlecolon'
+httpget ${HUGOTEST_BASEURL_0}p1/ 'failed'
+
+replace $WORK/content/p1/index.md 'titlecolon' 'title:'
+httpget ${HUGOTEST_BASEURL_0}p1/ 'Title: P1'
+
+stopServer
+
+-- hugo.toml --
+title = "Hugo Server Test"
+baseURL = "https://example.org/"
+disableKinds = ["taxonomy", "term", "sitemap"]
+-- layouts/index.html --
+Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
+-- layouts/_default/single.html --
+Title: {{ .Title }}|BaseURL: {{ site.BaseURL }}|
+-- content/_index.md --
+---
+title: Hugo Home
+---
+-- content/p1/index.md --
+---
+title: P1
+---
+-- content/p2/index.md --
+---
+title: P2
+---
+-- static/staticfiles/static.txt --
+static
+
+
+

tpl/template.go

@@ -40,7 +40,6 @@ type TemplateManager interface {
 	TemplateHandler
 	TemplateFuncGetter
 	AddTemplate(name, tpl string) error
-	MarkReady() error
 }
 
 // TemplateVariants describes the possible variants of a template.

tpl/tplimpl/template.go

@@ -168,6 +168,10 @@ func newTemplateHandlers(d *deps.Deps) (*tpl.TemplateHandlers, error) {
 		return nil, err
 	}
 
+	if err := h.main.createPrototypes(); err != nil {
+		return nil, err
+	}
+
 	e := &templateExec{
 		d:               d,
 		executor:        exec,
@@ -312,28 +316,11 @@ func (t *templateExec) GetFunc(name string) (reflect.Value, bool) {
 	return v, found
 }
 
-func (t *templateExec) MarkReady() error {
-	var err error
-	t.readyInit.Do(func() {
-		// We only need the clones if base templates are in use.
-		if len(t.needsBaseof) > 0 {
-			err = t.main.createPrototypes()
-			if err != nil {
-				return
-			}
-		}
-	})
-
-	return err
-}
-
 type templateHandler struct {
 	main        *templateNamespace
 	needsBaseof map[string]templateInfo
 	baseof      map[string]templateInfo
 
-	readyInit sync.Once
-
 	// This is the filesystem to load the templates from. All the templates are
 	// stored in the root of this filesystem.
 	layoutsFs afero.Fs