From 5df873a4e062fd93347927e77571e25e314dc245 Mon Sep 17 00:00:00 2001 From: David Glasser Date: Mon, 22 Dec 2014 12:19:59 -0800 Subject: [PATCH] Don't crash on pathwatcher failures Instead, increase polling interval for that file/dir to 500ms from 5s. Fixes #3336. The most common case where this was occuring is when you're on Linux and your inotify max_user_watches is too low. A wiki page will explain how to increase this. In debug mode, a message will tell you to go to the wiki page if you hit the limit (we may later expose this message by default, but it is part of the new user experience). Conflicts: meteor tools/safe-pathwatcher.js --- meteor | 2 +- scripts/generate-dev-bundle.sh | 6 +++- tools/safe-pathwatcher.js | 66 ++++++++++++++++++++++++++++------ tools/watch.js | 4 +++ 4 files changed, 65 insertions(+), 13 deletions(-) diff --git a/meteor b/meteor index 6e5eb77379..2b1f35547f 100755 --- a/meteor +++ b/meteor @@ -1,6 +1,6 @@ #!/bin/bash -BUNDLE_VERSION=0.3.74 +BUNDLE_VERSION=0.3.78 # OS Check. Put here because here is where we download the precompiled # bundles that are arch specific. diff --git a/scripts/generate-dev-bundle.sh b/scripts/generate-dev-bundle.sh index f67f143106..d2ad0ff957 100755 --- a/scripts/generate-dev-bundle.sh +++ b/scripts/generate-dev-bundle.sh @@ -121,7 +121,11 @@ cd "${DIR}/lib" # TODO Move this into dev-bundle-tool-package.js when it can be safely # installed that way (i.e. without build nan/runas build errors). -npm install pathwatcher@2.3.5 +# XXX This contains a patch to expose the errno from failed syscalls, so +# we can better understand why some users can't use pathwatcher. +# We have to install from the npm registry in order to get coffeescript +# output. The patch is https://github.com/atom/node-pathwatcher/pull/53 +npm install meteor-pathwatcher-tweaks@2.3.5 # Clean up some bulky stuff. cd node_modules diff --git a/tools/safe-pathwatcher.js b/tools/safe-pathwatcher.js index 6469e0a2e8..20402447c6 100644 --- a/tools/safe-pathwatcher.js +++ b/tools/safe-pathwatcher.js @@ -2,12 +2,16 @@ var fs = require("fs"); // Set this env variable to a truthy value to force fs.watchFile instead // of pathwatcher.watch. -var canUsePathwatcher = !process.env.METEOR_WATCH_FORCE_POLLING; +var PATHWATCHER_ENABLED = !process.env.METEOR_WATCH_FORCE_POLLING; -var pollingInterval = canUsePathwatcher - // Set this env variable to alter the watchFile polling interval. - ? ~~process.env.METEOR_WATCH_POLLING_INTERVAL_MS || 5000 - : 500; +var DEFAULT_POLLING_INTERVAL = + ~~process.env.METEOR_WATCH_POLLING_INTERVAL_MS || 5000; + + +var NO_PATHWATCHER_POLLING_INTERVAL = + ~~process.env.METEOR_WATCH_POLLING_INTERVAL_MS || 500; + +var suggestedRaisingWatchLimit = false; exports.watch = function watch(absPath, callback) { var lastPathwatcherEventTime = 0; @@ -23,8 +27,48 @@ exports.watch = function watch(absPath, callback) { callback.apply(this, arguments); } - var watcher = canUsePathwatcher && - require("pathwatcher").watch(absPath, pathwatcherWrapper); + var watcher = null; + if (PATHWATCHER_ENABLED) { + var pathwatcher = require('meteor-pathwatcher-tweaks'); + try { + watcher = pathwatcher.watch(absPath, pathwatcherWrapper); + } catch (e) { + // If it isn't an actual pathwatcher failure, rethrow. + if (e.message !== 'Unable to watch path') + throw e; + var constants = require('constants'); + var archinfo = require('./archinfo.js'); + if (! suggestedRaisingWatchLimit && + // Note: the not-super-documented require('constants') maps from + // strings to SYSTEM errno values. System errno values aren't the same + // as the numbers used internally by libuv! It would be nice to just + // make pathwatcher process the system errno into a string for us, but + // this is a pain, because posix doesn't give us a function to give us + // 'ENOSPC'-style strings (just the longer strings that strerror gives + // you). While libuv does give us uv_err_name, it takes in a *UV* + // errno value, which is different from the system errno value, and + // the translation function is not exposed: + // https://github.com/libuv/libuv/issues/79 + e.code === constants.ENOSPC && + // The only suggestion we currently have is for Linux. + archinfo.matches(archinfo.host(), 'os.linux')) { + suggestedRaisingWatchLimit = true; + var Console = require('./console.js').Console; + // XXX If we're comfortable with this message appearing in the new user + // experience, change it from debug to arrowWarn. + Console.debug( + "It looks like a simple tweak to your system's configuration will " + + "make many tools (including this meteor command) more efficient. " + + "To learn more, see " + + Console.url("https://github.com/meteor/meteor/wiki/File-Change-Watcher-Efficiency")); + } + // ... ignore the error. We'll still have watchFile, which is good + // enough. + } + } + + var pollingInterval = watcher + ? DEFAULT_POLLING_INTERVAL : NO_PATHWATCHER_POLLING_INTERVAL; function watchFileWrapper() { // If a pathwatcher event fired in the last polling interval, ignore @@ -34,10 +78,10 @@ exports.watch = function watch(absPath, callback) { } } - // We use fs.watchFile in addition to pathwatcher.watch as a fail-safe - // to detect file changes even on network file systems. However (unless - // canUsePathwatcher is false), we use a relatively long default polling - // interval of 5000ms to save CPU cycles. + // We use fs.watchFile in addition to pathwatcher.watch as a fail-safe to + // detect file changes even on network file systems. However (unless the user + // disabled pathwatcher or this pathwatcher call failed), we use a relatively + // long default polling interval of 5000ms to save CPU cycles. fs.watchFile(absPath, { persistent: false, interval: pollingInterval diff --git a/tools/watch.js b/tools/watch.js index 7293d6cd4e..4062507236 100644 --- a/tools/watch.js +++ b/tools/watch.js @@ -463,6 +463,8 @@ _.extend(Watcher.prototype, { if (stat === null || stat.isFile()) { if (_.has(self.watchSet.files, absPath)) { self._fireIfFileChanged(absPath); + // XXX #3335 We probably should check again in a second, due to low + // filesystem modtime resolution. } } else if (stat.isDirectory()) { @@ -500,6 +502,8 @@ _.extend(Watcher.prototype, { return self.stopped || (absPath === info.absPath && self._fireIfDirectoryChanged(info, true)); + // XXX #3335 We probably should check again in a second, due to low + // filesystem modtime resolution. }); } });