From 411ab817465e0153f4de909a817d48c2e3c8181c Mon Sep 17 00:00:00 2001 From: Kuisong Tong Date: Thu, 21 Nov 2024 10:27:37 -0800 Subject: [PATCH] eliminate potential double watching on a loader (#571) due to race condition, which is starting watching between loader has been added into providers and registering watch in Load. --- config.go | 35 +++++++++++++++++------------------ watch.go | 3 +++ 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/config.go b/config.go index f0488fe..8496ab5 100644 --- a/config.go +++ b/config.go @@ -72,18 +72,6 @@ func (c *Config) Load(loader Loader) error { } c.nocopy.Check() - // Load values into a new provider. - values, err := loader.Load() - if err != nil { - return fmt.Errorf("load configuration: %w", err) - } - c.transformKeys(values) - provider := c.providers.append(loader, values) - - if _, ok := loader.(Watcher); !ok { - return nil - } - // Register status callback if the loader is a Statuser. if statuser, ok := loader.(Statuser); ok { statuser.Status(func(changed bool, err error) { @@ -101,10 +89,20 @@ func (c *Config) Load(loader Loader) error { }) } - // Register watch callback if the loader is a Watcher and the watch is started. - // While Config.Watch is called, c.watched is set for registering the watch callback. - if watch := c.watched.Load(); watch != nil { - (*watch)(provider) + // Load values into a new provider. + values, err := loader.Load() + if err != nil { + return fmt.Errorf("load configuration: %w", err) + } + c.transformKeys(values) + provider := c.providers.append(loader, values) + + if _, ok := loader.(Watcher); ok { + // Register watch callback if the loader is a Watcher and the watch is started. + // While Config.Watch is called, c.watched is set for registering the watch callback. + if watch := c.watched.Load(); watch != nil { + (*watch)(provider) + } } return nil @@ -245,8 +243,9 @@ type ( mutex sync.RWMutex } provider struct { - loader Loader - values atomic.Pointer[map[string]any] + loader Loader + values atomic.Pointer[map[string]any] + watched atomic.Bool } ) diff --git a/watch.go b/watch.go index 9081d2f..6112f69 100644 --- a/watch.go +++ b/watch.go @@ -29,6 +29,9 @@ func (c *Config) Watch(ctx context.Context) error { //nolint:cyclop,funlen,gocog defer close(onChangesChannel) var waitGroup sync.WaitGroup watchProvider := func(provider *provider) { + if !provider.watched.CompareAndSwap(false, true) { + return // Skip if the provider has been watched. + } if watcher, ok := provider.loader.(Watcher); ok { waitGroup.Add(1) go func(ctx context.Context) {