Copilot commented on code in PR #2992:
URL: https://github.com/apache/dubbo-go/pull/2992#discussion_r2328396974
##########
loader.go:
##########
@@ -39,11 +43,15 @@ import (
import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/common/constant/file"
+ "dubbo.apache.org/dubbo-go/v3/common/extension"
)
var (
defaultActive = "default"
instanceOptions = defaultInstanceOptions()
+ once sync.Once
+ stopCh = make(chan struct{})
+ watcherWg sync.WaitGroup
Review Comment:
Global variables for concurrency control create potential race conditions.
The stopCh channel and watcherWg should be encapsulated within a struct or
managed differently to avoid shared mutable state across goroutines.
##########
loader.go:
##########
@@ -336,3 +416,11 @@ func checkPlaceholder(s string) (newKey, defaultValue
string) {
return
}
+
+// StopFileWatcher Stop file listener
+func StopFileWatcher() {
+ logger.Info("Stopping file watcher...")
+ close(stopCh)
+ watcherWg.Wait()
+ logger.Info("File watcher stopped successfully")
+}
Review Comment:
Closing stopCh multiple times will cause a panic. The function should check
if the channel is already closed or use a sync.Once to ensure it's only closed
once.
##########
loader.go:
##########
@@ -64,9 +72,81 @@ func Load(opts ...LoaderConfOption) error {
}
instance := &Instance{insOpts: instanceOptions}
+ // start the file watcher
+ once.Do(func() {
+ gr.GoSafely(&watcherWg, false, func() {
+ watch(conf, stopCh)
+ }, nil)
+ extension.AddCustomShutdownCallback(func() {
+ StopFileWatcher()
+ })
+ })
return instance.start()
}
+func watch(conf *loaderConf, stopCh <-chan struct{}) {
+ watcher, err := fsnotify.NewWatcher()
+ if err != nil {
+ logger.Errorf("Failed to initialize file watcher, error: %v",
err)
+ return
+ }
+ defer watcher.Close()
+
+ err = watcher.Add(conf.path)
+ if err != nil {
+ logger.Errorf("Failed to add file %s to watcher, error: %v",
conf.path, err)
+ return
+ }
+
+ for {
+ select {
+ case <-stopCh:
+ logger.Infof("File watcher is stopping...")
+ return
+ case event := <-watcher.Events:
+ if event.Op&fsnotify.Write == fsnotify.Write {
+ logger.Infof("Configuration file %s updated,
initiating hot reload...", event.Name)
+ if err := hotUpdateConfig(conf); err != nil {
+ logger.Warnf("Hot reload of
configuration failed, error: %v", err)
+ }
+ }
+ case err := <-watcher.Errors:
+ logger.Warnf("File watcher encountered an error: %v",
err)
+ }
+ }
+}
+
+func hotUpdateConfig(conf *loaderConf) error {
+ newOpts := defaultInstanceOptions()
+ bytes, err := os.ReadFile(conf.path)
+ if err != nil {
+ return err
+ }
+ conf.bytes = bytes
+
+ koan := GetConfigResolver(conf)
+ koan = conf.MergeConfig(koan)
+
+ if err := koan.UnmarshalWithConf(newOpts.Prefix(), newOpts,
koanf.UnmarshalConf{Tag: "yaml"}); err != nil {
+ return err
+ }
+
+ if err := newOpts.init(); err != nil {
+ return err
+ }
+
+ // Any part of the application that accesses instanceOptions directly
will now use the new values.
+ instanceOptions = newOpts
Review Comment:
Direct assignment to the global instanceOptions variable creates a race
condition. Multiple goroutines could be reading instanceOptions while it's
being updated, leading to inconsistent state. Consider using atomic operations
or proper synchronization mechanisms.
##########
loader.go:
##########
@@ -64,9 +72,81 @@ func Load(opts ...LoaderConfOption) error {
}
instance := &Instance{insOpts: instanceOptions}
+ // start the file watcher
+ once.Do(func() {
Review Comment:
The watcherWg.Add() should be called before starting the goroutine to avoid
race conditions where StopFileWatcher() could call Wait() before the goroutine
increments the counter.
```suggestion
once.Do(func() {
watcherWg.Add(1)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
