From a72f3a161c156c13f185df9fb87bf0d20240a49d Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sun, 17 Feb 2019 14:57:36 +0000 Subject: [PATCH] plugin/reload: fix data races (#2567) Reload didn't take proper care to protect the fields from use in different goroutines. Add a mutex and add helpers for usage and interval. Signed-off-by: Miek Gieben --- plugin/reload/reload.go | 42 +++++++++++++++++++++++++++++++++-------- plugin/reload/setup.go | 11 ++++++----- 2 files changed, 40 insertions(+), 13 deletions(-) diff --git a/plugin/reload/reload.go b/plugin/reload/reload.go index 8bb1c9884..3abc33835 100644 --- a/plugin/reload/reload.go +++ b/plugin/reload/reload.go @@ -2,6 +2,7 @@ package reload import ( "crypto/md5" + "sync" "time" "github.com/mholt/caddy" @@ -15,9 +16,34 @@ const ( ) type reload struct { - interval time.Duration - usage int - quit chan bool + dur time.Duration + u int + mtx sync.RWMutex + quit chan bool +} + +func (r *reload) setUsage(u int) { + r.mtx.Lock() + defer r.mtx.Unlock() + r.u = u +} + +func (r *reload) usage() int { + r.mtx.RLock() + defer r.mtx.RUnlock() + return r.u +} + +func (r *reload) setInterval(i time.Duration) { + r.mtx.Lock() + defer r.mtx.Unlock() + r.dur = i +} + +func (r *reload) interval() time.Duration { + r.mtx.RLock() + defer r.mtx.RUnlock() + return r.dur } func hook(event caddy.EventName, info interface{}) error { @@ -28,7 +54,7 @@ func hook(event caddy.EventName, info interface{}) error { // if reload is removed from the Corefile, then the hook // is still registered but setup is never called again // so we need a flag to tell us not to reload - if r.usage == unused { + if r.usage() == unused { return nil } @@ -38,7 +64,7 @@ func hook(event caddy.EventName, info interface{}) error { log.Infof("Running configuration MD5 = %x\n", md5sum) go func() { - tick := time.NewTicker(r.interval) + tick := time.NewTicker(r.interval()) for { select { @@ -53,15 +79,15 @@ func hook(event caddy.EventName, info interface{}) error { md5sum = s // now lets consider that plugin will not be reload, unless appear in next config file // change status iof usage will be reset in setup if the plugin appears in config file - r.usage = maybeUsed + r.setUsage(maybeUsed) _, err := instance.Restart(corefile) if err != nil { log.Errorf("Corefile changed but reload failed: %s", err) continue } // we are done, if the plugin was not set used, then it is not. - if r.usage == maybeUsed { - r.usage = unused + if r.usage() == maybeUsed { + r.setUsage(unused) } return } diff --git a/plugin/reload/setup.go b/plugin/reload/setup.go index c6b33e959..ed4d9f85d 100644 --- a/plugin/reload/setup.go +++ b/plugin/reload/setup.go @@ -25,9 +25,10 @@ func init() { // it is used to transmit data between Setup and start of the hook called 'onInstanceStartup' // channel for QUIT is never changed in purpose. // WARNING: this data may be unsync after an invalid attempt of reload Corefile. -var r = reload{interval: defaultInterval, usage: unused, quit: make(chan bool)} -var once sync.Once -var shutOnce sync.Once +var ( + r = reload{dur: defaultInterval, u: unused, quit: make(chan bool)} + once, shutOnce sync.Once +) func setup(c *caddy.Controller) error { c.Next() // 'reload' @@ -69,8 +70,8 @@ func setup(c *caddy.Controller) error { i = i + jitter // prepare info for next onInstanceStartup event - r.interval = i - r.usage = used + r.setInterval(i) + r.setUsage(used) once.Do(func() { caddy.RegisterEventHook("reload", hook)