dangling goroutine

[7d4b9]

Considering the following snippets (end and begin of function WatchConfig)

• END of WatchConfig

• BEGIN of WatchConfig

  

  It is obvious that init.Done() is called before eventsWG.Wait()

  This mean that the inner goroutine which process the events is DANGLING and cannot be stopped and checked.

So my Question is: Why making people believe that the design is standard with kind of "standard like/agnostic construct".

Lines initWG.Done() and eventWG.Wait() should be swapped to be ok but it is not possible without deadlock.

So to be more aligned with good readable goroutine

• Why using an external goroutine that is doing semi-waiting the inner one ? It is pointless Imho + introduces complexity to understand what's going on with the WatchConfig function.

So here is what I propose to do:

// WatchConfig watches and notifies changes on the file configFile.
func WatchConfig() { v.WatchConfig() }
func (v *Viper) WatchConfig() {
        // 👉 GOROUTINE 0
    // Make sure not to run twice this routine
    v.CancelWatchConfig()

    // we have to watch the entire directory to pick up renames/atomic saves in a cross-platform way
    filename, err := v.getConfigFile()
    if err != nil {
        log.Printf("error: %v\n", err)
        return
    }

    configFile := filepath.Clean(filename)
    configDir, _ := filepath.Split(configFile)
    realConfigFile, _ := filepath.EvalSymlinks(filename)

    watcher, err := fsnotify.NewWatcher()
    if err != nil {
        log.Fatal(err)
    }
    watcher.Add(configDir)

    // 👉cancellation and waiting point (may be a member of `Viper` object `v.cancelWatchConfig`)
    var watcherGroup sync.WaitGroup
    cancelWatchConfig  := func() {
        watcher.Close()
        watcherGroup.Wait()
    }
    // Process watcher events
    watcherGroup.Add(1)
    go func() {
                 // 👉 GOROUTINE 1
        defer watcherGroup.Done() 
        for event := range watcher.Events {
            currentConfigFile, _ := filepath.EvalSymlinks(filename)
            // we only care about the config file with the following cases:
            // 1 - if the config file was modified or created
            // 2 - if the real path to the config file changed (eg: k8s ConfigMap replacement)
            const writeOrCreateMask = fsnotify.Write | fsnotify.Create
            if (filepath.Clean(event.Name) == configFile &&
                event.Op&writeOrCreateMask != 0) ||
                (currentConfigFile != "" && currentConfigFile != realConfigFile) {
                realConfigFile = currentConfigFile
                err := v.ReadInConfig()
                if err != nil {
                    log.Printf("error reading config file: %v\n", err)
                }
                if v.onConfigChange != nil {
                    v.onConfigChange(event)
                }
            } else if filepath.Clean(event.Name) == configFile &&
                event.Op&fsnotify.Remove&fsnotify.Remove != 0 {
                return
            }
        }
    }()
    // Process watcher errors
    watcherGroup.Add(1)
    go func() {
                 // 👉 GOROUTINE 1 (Bis)
        defer watcherGroup.Done()
        for err := range watcher.Errors {
            log.Printf("watcher error: %v\n", err)
        }
    }()
}

This appear to be far away better as for now there is no more dangling and it is now POSSIBLE to cancel the whole watching with cancelWatchConfig

[7d4b9]

837 Resolves This

[7d4b9]

This also allows one to call WriteConfig while WatchConfig have been called already because now the WriteConfig can cancel and restart the watching of config file with no race conditions on file access (data corruption) This is resolving #742