fix: edge cases caused file changes to stop being detected (#4749)

Lock was not released if the loop did a `continue` (such as if there was a current compilation occuring). Moved the locked part of the loop into a function so we can use `defer` to release the lock
block · Mar 3, 2025 · db85da8 · db85da8
1 parent 32f3de4
commit db85da8
Showing 1 changed file with 49 additions and 43 deletions.
diff --git a/internal/watch/watch.go b/internal/watch/watch.go
@@ -161,53 +161,59 @@ func (w *Watcher) Watch(ctx context.Context, period time.Duration, moduleDirs []
 				return config.Dir, config
 			})
 
-			w.mutex.Lock()
-			// Trigger events for removed modules.
-			for _, existingModule := range w.existingModules {
-				if transactions, ok := w.moduleTransactions[existingModule.Config.Dir]; ok && len(transactions) > 0 {
-					// Skip modules that currently have transactions
-					continue
-				}
-				existingConfig := existingModule.Config
-				if _, haveModule := modulesByDir[existingConfig.Dir]; !haveModule {
-					logger.Debugf("removed %q", existingModule.Config.Module)
-					topic.Publish(WatchEventModuleRemoved{Config: existingModule.Config})
-					delete(w.existingModules, existingConfig.Dir)
-				}
-			}
+			w.detectChanges(ctx, topic, modulesByDir)
+		}
+	}()
+	return topic, nil
+}
 
-			// Compare the modules to the existing modules.
-			for _, config := range modulesByDir {
-				if transactions, ok := w.moduleTransactions[config.Dir]; ok && len(transactions) > 0 {
-					// Skip modules that currently have transactions
-					continue
-				}
-				existingModule, haveExistingModule := w.existingModules[config.Dir]
-				hashes, err := ComputeFileHashes(config.Dir, true, w.patterns)
-				if err != nil {
-					logger.Tracef("error computing file hashes for %s: %v", config.Dir, err)
-					continue
-				}
+func (w *Watcher) detectChanges(ctx context.Context, topic *pubsub.Topic[WatchEvent], modulesByDir map[string]moduleconfig.UnvalidatedModuleConfig) {
+	logger := log.FromContext(ctx)
+	w.mutex.Lock()
+	defer w.mutex.Unlock()
 
-				if haveExistingModule {
-					changes := CompareFileHashes(existingModule.Hashes, hashes)
-					if len(changes) == 0 {
-						continue
-					}
-					event := WatchEventModuleChanged{Config: existingModule.Config, Changes: changes, Time: time.Now()}
-					logger.Debugf("changed %q: %s", config.Module, event)
-					topic.Publish(event)
-					w.existingModules[config.Dir] = moduleHashes{Hashes: hashes, Config: existingModule.Config}
-					continue
-				}
-				logger.Debugf("added %q", config.Module)
-				topic.Publish(WatchEventModuleAdded{Config: config})
-				w.existingModules[config.Dir] = moduleHashes{Hashes: hashes, Config: config}
+	// Trigger events for removed modules.
+	for _, existingModule := range w.existingModules {
+		if transactions, ok := w.moduleTransactions[existingModule.Config.Dir]; ok && len(transactions) > 0 {
+			// Skip modules that currently have transactions
+			continue
+		}
+		existingConfig := existingModule.Config
+		if _, haveModule := modulesByDir[existingConfig.Dir]; !haveModule {
+			logger.Debugf("removed %q", existingModule.Config.Module)
+			topic.Publish(WatchEventModuleRemoved{Config: existingModule.Config})
+			delete(w.existingModules, existingConfig.Dir)
+		}
+	}
+
+	// Compare the modules to the existing modules.
+	for _, config := range modulesByDir {
+		if transactions, ok := w.moduleTransactions[config.Dir]; ok && len(transactions) > 0 {
+			// Skip modules that currently have transactions
+			continue
+		}
+		existingModule, haveExistingModule := w.existingModules[config.Dir]
+		hashes, err := ComputeFileHashes(config.Dir, true, w.patterns)
+		if err != nil {
+			logger.Tracef("error computing file hashes for %s: %v", config.Dir, err)
+			continue
+		}
+
+		if haveExistingModule {
+			changes := CompareFileHashes(existingModule.Hashes, hashes)
+			if len(changes) == 0 {
+				continue
 			}
-			w.mutex.Unlock()
+			event := WatchEventModuleChanged{Config: existingModule.Config, Changes: changes, Time: time.Now()}
+			logger.Debugf("changed %q: %s", config.Module, event)
+			topic.Publish(event)
+			w.existingModules[config.Dir] = moduleHashes{Hashes: hashes, Config: existingModule.Config}
+			continue
 		}
-	}()
-	return topic, nil
+		logger.Debugf("added %q", config.Module)
+		topic.Publish(WatchEventModuleAdded{Config: config})
+		w.existingModules[config.Dir] = moduleHashes{Hashes: hashes, Config: config}
+	}
 }
 
 // ModifyFilesTransaction allows builds to modify files in a module without triggering a watch event.