commands: Fix data race

By wrapping all use of the shared config in a lock.

Updates #10953
This commit is contained in:
Bjørn Erik Pedersen 2023-05-19 12:54:42 +02:00
parent c371171ab8
commit 0a51dfac9e
3 changed files with 215 additions and 155 deletions

View file

@ -79,18 +79,12 @@ func Execute(args []string) error {
}
type commonConfig struct {
mu sync.Mutex
mu *sync.Mutex
configs *allconfig.Configs
cfg config.Provider
fs *hugofs.Fs
}
func (c *commonConfig) getFs() *hugofs.Fs {
c.mu.Lock()
defer c.mu.Unlock()
return c.fs
}
// This is the root command.
type rootCommand struct {
Printf func(format string, v ...interface{})
@ -181,6 +175,7 @@ func (r *rootCommand) ConfigFromConfig(key int32, oldConf *commonConfig) (*commo
}
return &commonConfig{
mu: oldConf.mu,
configs: configs,
cfg: oldConf.cfg,
fs: fs,
@ -295,6 +290,7 @@ func (r *rootCommand) ConfigFromProvider(key int32, cfg config.Provider) (*commo
}
commonConfig := &commonConfig{
mu: &sync.Mutex{},
configs: configs,
cfg: cfg,
fs: fs,
@ -309,9 +305,6 @@ func (r *rootCommand) ConfigFromProvider(key int32, cfg config.Provider) (*commo
func (r *rootCommand) HugFromConfig(conf *commonConfig) (*hugolib.HugoSites, error) {
h, _, err := r.hugoSites.GetOrCreate(r.configVersionID.Load(), func(key int32) (*hugolib.HugoSites, error) {
conf.mu.Lock()
defer conf.mu.Unlock()
depsCfg := deps.DepsCfg{Configs: conf.configs, Fs: conf.fs, Logger: r.logger}
return hugolib.NewHugoSites(depsCfg)
})

View file

@ -52,8 +52,8 @@ import (
type hugoBuilder struct {
r *rootCommand
cunfMu sync.Mutex
conf_ *commonConfig
confmu sync.Mutex
conf *commonConfig
// May be nil.
s *serverCommand
@ -74,16 +74,17 @@ type hugoBuilder struct {
errState hugoBuilderErrState
}
func (c *hugoBuilder) conf() *commonConfig {
c.cunfMu.Lock()
defer c.cunfMu.Unlock()
return c.conf_
func (c *hugoBuilder) withConfE(fn func(conf *commonConfig) error) error {
c.confmu.Lock()
defer c.confmu.Unlock()
return fn(c.conf)
}
func (c *hugoBuilder) setConf(conf *commonConfig) {
c.cunfMu.Lock()
defer c.cunfMu.Unlock()
c.conf_ = conf
func (c *hugoBuilder) withConf(fn func(conf *commonConfig)) {
c.confmu.Lock()
defer c.confmu.Unlock()
fn(c.conf)
}
type hugoBuilderErrState struct {
@ -357,7 +358,10 @@ func (c *hugoBuilder) newWatcher(pollIntervalStr string, dirList ...string) (*wa
// Identifies changes to config (config.toml) files.
configSet := make(map[string]bool)
configFiles := c.conf().configs.LoadingInfo.ConfigFiles
var configFiles []string
c.withConf(func(conf *commonConfig) {
configFiles = conf.configs.LoadingInfo.ConfigFiles
})
c.r.logger.Println("Watching for config changes in", strings.Join(configFiles, ", "))
for _, configFile := range configFiles {
@ -466,14 +470,18 @@ func (c *hugoBuilder) copyStaticTo(sourceFs *filesystems.SourceFilesystem) (uint
fs := &countingStatFs{Fs: sourceFs.Fs}
syncer := fsync.NewSyncer()
syncer.NoTimes = c.conf().configs.Base.NoTimes
syncer.NoChmod = c.conf().configs.Base.NoChmod
c.withConf(func(conf *commonConfig) {
syncer.NoTimes = conf.configs.Base.NoTimes
syncer.NoChmod = conf.configs.Base.NoChmod
syncer.ChmodFilter = chmodFilter
syncer.SrcFs = fs
syncer.DestFs = c.conf().fs.PublishDirStatic
syncer.DestFs = conf.fs.PublishDirStatic
// Now that we are using a unionFs for the static directories
// We can effectively clean the publishDir on initial sync
syncer.Delete = c.conf().configs.Base.CleanDestinationDir
syncer.Delete = conf.configs.Base.CleanDestinationDir
})
syncer.SrcFs = fs
if syncer.Delete {
c.r.logger.Infoln("removing all files from destination that don't exist in static dirs")
@ -518,9 +526,11 @@ func (c *hugoBuilder) doWithPublishDirs(f func(sourceFs *filesystems.SourceFiles
}
if lang == "" {
// Not multihost
for _, l := range c.conf().configs.Languages {
c.withConf(func(conf *commonConfig) {
for _, l := range conf.configs.Languages {
langCount[l.Lang] = cnt
}
})
} else {
langCount[lang] = cnt
}
@ -562,7 +572,11 @@ func (c *hugoBuilder) fullBuild(noBuildLock bool) error {
// Do not copy static files and build sites in parallel if cleanDestinationDir is enabled.
// This flag deletes all static resources in /public folder that are missing in /static,
// and it does so at the end of copyStatic() call.
if c.conf().configs.Base.CleanDestinationDir {
var cleanDestinationDir bool
c.withConf(func(conf *commonConfig) {
cleanDestinationDir = conf.configs.Base.CleanDestinationDir
})
if cleanDestinationDir {
if err := copyStaticFunc(); err != nil {
return err
}
@ -692,8 +706,8 @@ func (c *hugoBuilder) handleEvents(watcher *watcher.Batcher,
}
if ev.Op&fsnotify.Remove == fsnotify.Remove || ev.Op&fsnotify.Rename == fsnotify.Rename {
configFiles := c.conf().configs.LoadingInfo.ConfigFiles
for _, configFile := range configFiles {
c.withConf(func(conf *commonConfig) {
for _, configFile := range conf.configs.LoadingInfo.ConfigFiles {
counter := 0
for watcher.Add(configFile) != nil {
counter++
@ -703,6 +717,7 @@ func (c *hugoBuilder) handleEvents(watcher *watcher.Batcher,
time.Sleep(100 * time.Millisecond)
}
}
})
}
// Config file(s) changed. Need full rebuild.
@ -834,9 +849,11 @@ func (c *hugoBuilder) handleEvents(watcher *watcher.Batcher,
// recursively add new directories to watch list
// When mkdir -p is used, only the top directory triggers an event (at least on OSX)
if ev.Op&fsnotify.Create == fsnotify.Create {
if s, err := c.conf().fs.Source.Stat(ev.Name); err == nil && s.Mode().IsDir() {
_ = helpers.SymbolicWalk(c.conf().fs.Source, ev.Name, walkAdder)
c.withConf(func(conf *commonConfig) {
if s, err := conf.fs.Source.Stat(ev.Name); err == nil && s.Mode().IsDir() {
_ = helpers.SymbolicWalk(conf.fs.Source, ev.Name, walkAdder)
}
})
}
if staticSyncer.isStatic(h, ev.Name) {
@ -942,10 +959,16 @@ func (c *hugoBuilder) handleEvents(watcher *watcher.Batcher,
}
func (c *hugoBuilder) hugo() (*hugolib.HugoSites, error) {
h, err := c.r.HugFromConfig(c.conf())
if err != nil {
var h *hugolib.HugoSites
if err := c.withConfE(func(conf *commonConfig) error {
var err error
h, err = c.r.HugFromConfig(conf)
return err
}); err != nil {
return nil, err
}
if c.s != nil {
// A running server, register the media types.
for _, s := range h.Sites {
@ -956,7 +979,10 @@ func (c *hugoBuilder) hugo() (*hugolib.HugoSites, error) {
}
func (c *hugoBuilder) hugoTry() *hugolib.HugoSites {
h, _ := c.r.HugFromConfig(c.conf())
var h *hugolib.HugoSites
c.withConf(func(conf *commonConfig) {
h, _ = c.r.HugFromConfig(conf)
})
return h
}
@ -977,7 +1003,7 @@ func (c *hugoBuilder) loadConfig(cd *simplecobra.Commandeer, running bool) error
return err
}
c.setConf(conf)
c.conf = conf
if c.onConfigLoaded != nil {
if err := c.onConfigLoaded(false); err != nil {
return err
@ -1014,8 +1040,9 @@ func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
return err
}
if c.fastRenderMode {
c.withConf(func(conf *commonConfig) {
// Make sure we always render the home pages
for _, l := range c.conf().configs.Languages {
for _, l := range conf.configs.Languages {
langPath := h.GetLangSubDir(l.Lang)
if langPath != "" {
langPath = langPath + "/"
@ -1023,6 +1050,7 @@ func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
home := h.PrependBasePath("/"+langPath, false)
visited[home] = true
}
})
}
return h.Build(hugolib.BuildCfg{NoBuildLock: true, RecentlyVisited: visited, ErrRecovery: c.errState.wasErr()}, events...)
}
@ -1030,18 +1058,25 @@ func (c *hugoBuilder) rebuildSites(events []fsnotify.Event) error {
func (c *hugoBuilder) reloadConfig() error {
c.r.Reset()
c.r.configVersionID.Add(1)
oldConf := c.conf()
conf, err := c.r.ConfigFromConfig(c.r.configVersionID.Load(), c.conf())
if err := c.withConfE(func(conf *commonConfig) error {
oldConf := conf
newConf, err := c.r.ConfigFromConfig(c.r.configVersionID.Load(), conf)
if err != nil {
return err
}
sameLen := len(oldConf.configs.Languages) == len(conf.configs.Languages)
sameLen := len(oldConf.configs.Languages) == len(newConf.configs.Languages)
if !sameLen {
if oldConf.configs.IsMultihost || conf.configs.IsMultihost {
if oldConf.configs.IsMultihost || newConf.configs.IsMultihost {
return errors.New("multihost change detected, please restart server")
}
}
c.setConf(conf)
c.conf = newConf
return nil
}); err != nil {
return err
}
if c.onConfigLoaded != nil {
if err := c.onConfigLoaded(true); err != nil {
return err

View file

@ -44,6 +44,7 @@ import (
"github.com/gohugoio/hugo/common/hugo"
"github.com/gohugoio/hugo/common/types"
"github.com/gohugoio/hugo/common/urls"
"github.com/gohugoio/hugo/config"
"github.com/gohugoio/hugo/helpers"
"github.com/gohugoio/hugo/hugofs"
"github.com/gohugoio/hugo/hugofs/files"
@ -197,7 +198,6 @@ type fileServer struct {
func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string, string, error) {
r := f.c.r
conf := f.c.conf()
baseURL := f.baseURLs[i]
root := f.roots[i]
port := f.c.serverPorts[i].p
@ -216,7 +216,11 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
}
}
httpFs := afero.NewHttpFs(conf.fs.PublishDirServer)
var httpFs *afero.HttpFs
f.c.withConf(func(conf *commonConfig) {
httpFs = afero.NewHttpFs(conf.fs.PublishDirServer)
})
fs := filesOnlyFs{httpFs.Dir(path.Join("/", root))}
if i == 0 && f.c.fastRenderMode {
r.Println("Running in Fast Render Mode. For full rebuilds on change: hugo server --disableFastRender")
@ -242,9 +246,11 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
}
port = 1313
f.c.withConf(func(conf *commonConfig) {
if lrport := conf.configs.GetFirstLanguageConfig().BaseURLLiveReload().Port(); lrport != 0 {
port = lrport
}
})
lr := *u
lr.Host = fmt.Sprintf("%s:%d", lr.Hostname(), port)
fmt.Fprint(w, injectLiveReloadScript(r, lr))
@ -258,7 +264,10 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
w.Header().Set("Pragma", "no-cache")
}
serverConfig := f.c.conf().configs.Base.Server
var serverConfig config.Server
f.c.withConf(func(conf *commonConfig) {
serverConfig = conf.configs.Base.Server
})
// Ignore any query params for the operations below.
requestURI, _ := url.PathUnescape(strings.TrimSuffix(r.RequestURI, "?"+r.URL.RawQuery))
@ -277,7 +286,10 @@ func (f *fileServer) createEndpoint(i int) (*http.ServeMux, net.Listener, string
if root != "" {
path = filepath.Join(root, path)
}
fs := f.c.conf().getFs().PublishDir
var fs afero.Fs
f.c.withConf(func(conf *commonConfig) {
fs = conf.fs.PublishDir
})
fi, err := fs.Stat(path)
@ -519,8 +531,10 @@ func (c *serverCommand) PreRun(cd, runner *simplecobra.Commandeer) error {
}
if !reloaded && c.fastRenderMode {
c.conf().fs.PublishDir = hugofs.NewHashingFs(c.conf().fs.PublishDir, c.changeDetector)
c.conf().fs.PublishDirStatic = hugofs.NewHashingFs(c.conf().fs.PublishDirStatic, c.changeDetector)
c.withConf(func(conf *commonConfig) {
conf.fs.PublishDir = hugofs.NewHashingFs(conf.fs.PublishDir, c.changeDetector)
conf.fs.PublishDirStatic = hugofs.NewHashingFs(conf.fs.PublishDirStatic, c.changeDetector)
})
}
return nil
@ -562,18 +576,19 @@ func (c *serverCommand) setBaseURLsInConfig() error {
if len(c.serverPorts) == 0 {
panic("no server ports set")
}
isMultiHost := c.conf().configs.IsMultihost
for i, language := range c.conf().configs.Languages {
return c.withConfE(func(conf *commonConfig) error {
for i, language := range conf.configs.Languages {
isMultiHost := conf.configs.IsMultihost
var serverPort int
if isMultiHost {
serverPort = c.serverPorts[i].p
} else {
serverPort = c.serverPorts[0].p
}
langConfig := c.conf().configs.LanguageConfigMap[language.Lang]
langConfig := conf.configs.LanguageConfigMap[language.Lang]
baseURLStr, err := c.fixURL(langConfig.BaseURL, c.r.baseURL, serverPort)
if err != nil {
return nil
return err
}
baseURL, err := urls.NewBaseURLFromString(baseURLStr)
if err != nil {
@ -587,6 +602,7 @@ func (c *serverCommand) setBaseURLsInConfig() error {
langConfig.C.SetBaseURL(baseURL, baseURLLiveReload)
}
return nil
})
}
func (c *serverCommand) getErrorWithContext() any {
@ -609,13 +625,16 @@ func (c *serverCommand) getErrorWithContext() any {
func (c *serverCommand) createServerPorts(cd *simplecobra.Commandeer) error {
flags := cd.CobraCommand.Flags()
isMultiHost := c.conf().configs.IsMultihost
var cerr error
c.withConf(func(conf *commonConfig) {
isMultiHost := conf.configs.IsMultihost
c.serverPorts = make([]serverPortListener, 1)
if isMultiHost {
if !c.serverAppend {
return errors.New("--appendPort=false not supported when in multihost mode")
cerr = errors.New("--appendPort=false not supported when in multihost mode")
return
}
c.serverPorts = make([]serverPortListener, len(c.conf().configs.Languages))
c.serverPorts = make([]serverPortListener, len(conf.configs.Languages))
}
currentServerPort := c.serverPort
for i := 0; i < len(c.serverPorts); i++ {
@ -625,19 +644,23 @@ func (c *serverCommand) createServerPorts(cd *simplecobra.Commandeer) error {
} else {
if i == 0 && flags.Changed("port") {
// port set explicitly by user -- he/she probably meant it!
return fmt.Errorf("server startup failed: %s", err)
cerr = fmt.Errorf("server startup failed: %s", err)
return
}
c.r.Println("port", currentServerPort, "already in use, attempting to use an available port")
l, sp, err := helpers.TCPListen()
if err != nil {
return fmt.Errorf("unable to find alternative port to use: %s", err)
cerr = fmt.Errorf("unable to find alternative port to use: %s", err)
return
}
c.serverPorts[i] = serverPortListener{ln: l, p: sp.Port}
}
currentServerPort = c.serverPorts[i].p + 1
}
return nil
})
return cerr
}
// fixURL massages the baseURL into a form needed for serving
@ -709,30 +732,37 @@ func (c *serverCommand) partialReRender(urls ...string) error {
}
func (c *serverCommand) serve() error {
isMultiHost := c.conf().configs.IsMultihost
var err error
h, err := c.r.HugFromConfig(c.conf())
if err != nil {
return err
}
r := c.r
var (
baseURLs []string
roots []string
h *hugolib.HugoSites
)
err := c.withConfE(func(conf *commonConfig) error {
isMultiHost := conf.configs.IsMultihost
var err error
h, err = c.r.HugFromConfig(conf)
if err != nil {
return err
}
if isMultiHost {
for _, l := range c.conf().configs.ConfigLangs() {
for _, l := range conf.configs.ConfigLangs() {
baseURLs = append(baseURLs, l.BaseURL().String())
roots = append(roots, l.Language().Lang)
}
} else {
l := c.conf().configs.GetFirstLanguageConfig()
l := conf.configs.GetFirstLanguageConfig()
baseURLs = []string{l.BaseURL().String()}
roots = []string{""}
}
return nil
})
if err != nil {
return err
}
// Cache it here. The HugoSites object may be unavailable later on due to intermittent configuration errors.
// To allow the en user to change the error template while the server is running, we use
// the freshest template we can provide.
@ -796,7 +826,7 @@ func (c *serverCommand) serve() error {
mu.HandleFunc(u.Path+"/livereload.js", livereload.ServeJS)
mu.HandleFunc(u.Path+"/livereload", livereload.Handler)
}
r.Printf("Web Server is available at %s (bind address %s)\n", serverURL, c.serverInterface)
c.r.Printf("Web Server is available at %s (bind address %s)\n", serverURL, c.serverInterface)
wg1.Go(func() error {
err = srv.Serve(listener)
if err != nil && err != http.ErrServerClosed {
@ -829,7 +859,7 @@ func (c *serverCommand) serve() error {
}
r.Println("Press Ctrl+C to stop")
c.r.Println("Press Ctrl+C to stop")
err = func() error {
for {
@ -845,7 +875,7 @@ func (c *serverCommand) serve() error {
}()
if err != nil {
r.Println("Error:", err)
c.r.Println("Error:", err)
}
if h := c.hugoTry(); h != nil {
@ -892,17 +922,17 @@ func (s *staticSyncer) syncsStaticEvents(staticEvents []fsnotify.Event) error {
publishDir = filepath.Join(publishDir, sourceFs.PublishFolder)
}
conf := s.c.conf().configs.Base
fs := s.c.conf().fs
syncer := fsync.NewSyncer()
syncer.NoTimes = conf.NoTimes
syncer.NoChmod = conf.NoChmod
c.withConf(func(conf *commonConfig) {
syncer.NoTimes = conf.configs.Base.NoTimes
syncer.NoChmod = conf.configs.Base.NoChmod
syncer.ChmodFilter = chmodFilter
syncer.SrcFs = sourceFs.Fs
syncer.DestFs = fs.PublishDir
syncer.DestFs = conf.fs.PublishDir
if c.s != nil && c.s.renderStaticToDisk {
syncer.DestFs = fs.PublishDirStatic
syncer.DestFs = conf.fs.PublishDirStatic
}
})
// prevent spamming the log on changes
logger := helpers.NewDistinctErrorLogger()
@ -946,7 +976,9 @@ func (s *staticSyncer) syncsStaticEvents(staticEvents []fsnotify.Event) error {
if _, err := sourceFs.Fs.Stat(relPath); herrors.IsNotExist(err) {
// If file doesn't exist in any static dir, remove it
logger.Println("File no longer exists in static dir, removing", relPath)
_ = c.conf().fs.PublishDirStatic.RemoveAll(relPath)
c.withConf(func(conf *commonConfig) {
_ = conf.fs.PublishDirStatic.RemoveAll(relPath)
})
} else if err == nil {
// If file still exists, sync it