From b950823cb24d4614bc4dc04b53e208ade6342130 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Fri, 21 Jun 2019 15:36:13 +0200 Subject: [PATCH] fix(core): chokidar watcher on mac could segfault after reloading configs 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 https://github.com/fsevents/fsevents/issues/273 for a discussion on this issue. --- garden-service/src/garden.ts | 7 ++- garden-service/src/util/util.ts | 2 +- garden-service/src/watch.ts | 67 ++++++++++++++-------------- garden-service/test/mocha.integ.opts | 1 + garden-service/test/mocha.opts | 1 + 5 files changed, 40 insertions(+), 38 deletions(-) diff --git a/garden-service/src/garden.ts b/garden-service/src/garden.ts index 13904b7075..d865154dbe 100644 --- a/garden-service/src/garden.ts +++ b/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) { diff --git a/garden-service/src/util/util.ts b/garden-service/src/util/util.ts index 32befd5b76..ecf7ff3e09 100644 --- a/garden-service/src/util/util.ts +++ b/garden-service/src/util/util.ts @@ -50,7 +50,7 @@ export type Unpacked = 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) } diff --git a/garden-service/src/watch.ts b/garden-service/src/watch.ts index 22ce066a26..8be89debd9 100644 --- a/garden-service/src/watch.ts +++ b/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 @@ -27,33 +46,21 @@ 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)) @@ -61,18 +68,12 @@ export class Watcher { .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() } } diff --git a/garden-service/test/mocha.integ.opts b/garden-service/test/mocha.integ.opts index 43dceca99f..879b244445 100644 --- a/garden-service/test/mocha.integ.opts +++ b/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 diff --git a/garden-service/test/mocha.opts b/garden-service/test/mocha.opts index 3d9c28197e..fce1bcc168 100644 --- a/garden-service/test/mocha.opts +++ b/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