Skip to content

Commit

Permalink
eliminate potential double watching on a loader (#571)
Browse files Browse the repository at this point in the history
due to race condition, which is starting watching between loader has
been added into providers and registering watch in Load.
  • Loading branch information
ktong authored Nov 21, 2024
1 parent 37bc6a3 commit 411ab81
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
35 changes: 17 additions & 18 deletions config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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
}
)

Expand Down
3 changes: 3 additions & 0 deletions watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down

0 comments on commit 411ab81

Please sign in to comment.