fix(core): chokidar watcher on mac could segfault after reloading con…

…figs

Turns out chokidar and the native fsevents module don't like it when multiple
instances are created in a single process, even if one is closed before the
other starts. Fixed it by making a global instance and reconfiguring on
reload instead. See fsevents/fsevents#273 for
a discussion on this issue.

garden-service/src/garden.ts

@@ -106,7 +106,7 @@ export class Garden {
   private modulesScanned: boolean
   private readonly registeredPlugins: { [key: string]: PluginFactory }
   private readonly taskGraph: TaskGraph
-  private readonly watcher: Watcher
+  private watcher: Watcher
 
   public readonly configStore: ConfigStore
   public readonly globalConfigStore: GlobalConfigStore
@@ -167,7 +167,6 @@ export class Garden {
 
     this.taskGraph = new TaskGraph(this, this.log)
     this.events = new EventBus(this.log)
-    this.watcher = new Watcher(this, this.log)
 
     // Register plugins
     for (const [name, pluginFactory] of Object.entries({ ...builtinPlugins, ...params.plugins })) {
@@ -234,7 +233,7 @@ export class Garden {
    * Clean up before shutting down.
    */
   async close() {
-    this.watcher.stop()
+    this.watcher && this.watcher.stop()
   }
 
   getPluginContext(providerName: string) {
@@ -255,7 +254,7 @@ export class Garden {
    */
   async startWatcher(graph: ConfigGraph) {
     const modules = await graph.getModules()
-    this.watcher.start(modules)
+    this.watcher = new Watcher(this, this.log, modules)
   }
 
   private registerPlugin(name: string, moduleOrFactory: RegisterPluginParam) {

garden-service/src/util/util.ts

@@ -50,7 +50,7 @@ export type Unpacked<T> =
 
 const MAX_BUFFER_SIZE = 1024 * 1024
 
-export function shutdown(code) {
+export function shutdown(code?: number) {
   // This is a good place to log exitHookNames if needed.
   process.exit(code)
 }

garden-service/src/watch.ts

@@ -16,7 +16,26 @@ import * as klaw from "klaw"
 import { registerCleanupFunction } from "./util/util"
 import * as Bluebird from "bluebird"
 import { some } from "lodash"
-import { isConfigFilename } from "./util/fs"
+import { isConfigFilename, Ignorer } from "./util/fs"
+
+// IMPORTANT: We must use a single global instance of the watcher, because we may otherwise get
+// segmentation faults on macOS! See https://github.com/fsevents/fsevents/issues/273
+let watcher: FSWatcher | undefined
+let ignorer: Ignorer
+let projectRoot: string
+
+const ignored = (path: string, _: any) => {
+  const relpath = relative(projectRoot, path)
+  return relpath && ignorer.ignores(relpath)
+}
+
+// The process hangs after tests if we don't do this
+registerCleanupFunction("stop watcher", () => {
+  if (watcher) {
+    watcher.close()
+    watcher = undefined
+  }
+})
 
 export type ChangeHandler = (module: Module | null, configChanged: boolean) => Promise<void>
 
@@ -27,52 +46,34 @@ export type ChangeHandler = (module: Module | null, configChanged: boolean) => P
 export class Watcher {
   private watcher: FSWatcher
 
-  constructor(private garden: Garden, private log: LogEntry) {
-  }
-
-  /**
-   * Starts the file watcher. Idempotent.
-   *
-   * @param modules All configured modules in the project.
-   */
-  start(modules: Module[]) {
-    // Only run one watcher for the process
-    if (this.watcher) {
-      return
-    }
-
-    const projectRoot = this.garden.projectRoot
-    const ignorer = this.garden.ignorer
+  constructor(private garden: Garden, private log: LogEntry, modules: Module[]) {
+    projectRoot = this.garden.projectRoot
+    ignorer = this.garden.ignorer
 
     this.log.debug(`Watcher: Watching ${projectRoot}`)
 
-    this.watcher = watch(projectRoot, {
-      ignored: (path: string, _: any) => {
-        const relpath = relative(projectRoot, path)
-        return relpath && ignorer.ignores(relpath)
-      },
-      ignoreInitial: true,
-      persistent: true,
-    })
+    if (watcher === undefined) {
+      watcher = watch(projectRoot, {
+        ignored,
+        ignoreInitial: true,
+        persistent: true,
+      })
+    }
+
+    this.watcher = watcher
 
     this.watcher
       .on("add", this.makeFileAddedHandler(modules))
       .on("change", this.makeFileChangedHandler("modified", modules))
       .on("unlink", this.makeFileChangedHandler("removed", modules))
       .on("addDir", this.makeDirAddedHandler(modules))
       .on("unlinkDir", this.makeDirRemovedHandler(modules))
-
-    registerCleanupFunction("clearFileWatches", () => {
-      this.stop()
-    })
   }
 
   stop(): void {
     if (this.watcher) {
-      this.log.debug(`Watcher: Stopping`)
-
-      this.watcher.close()
-      delete this.watcher
+      this.log.debug(`Watcher: Clearing handlers`)
+      this.watcher.removeAllListeners()
     }
   }
 

garden-service/test/mocha.integ.opts

@@ -5,5 +5,6 @@
 --watch-extensions js,json,pegjs
 --reporter spec
 --timeout 0
+--exit
 build/test/setup.js
 build/test/integ/src/**/*.js

garden-service/test/mocha.opts

@@ -5,5 +5,6 @@
 --watch-extensions js,json,pegjs
 --reporter spec
 --timeout 20000
+--exit
 build/test/setup.js
 build/test/unit/**/*.js