1berry/nodejs
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

src: make FSEventWrap/StatWatcher::Start more robust

Browse Source 
PR-URL: https://github.com/nodejs/node/pull/17432
Fixes: https://github.com/nodejs/node/issues/17430
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit is contained in:
Timothy Gu 2017-12-02 18:37:56 -08:00 committed by Anna Henningsen
parent 11ebaff15c
commit cd71fc1545
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
4 changed files with 35 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/fs_event_wrap.cc
View File

@ -111,7 +111,8 @@ void FSEventWrap::Start(const FunctionCallbackInfo<Value>& args) {
FSEventWrap* wrap; FSEventWrap* wrap;
ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder()); ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
CHECK_EQ(wrap->initialized_, false); if (wrap->initialized_)
return args.GetReturnValue().Set(0);
static const char kErrMsg[] = "filename must be a string or Buffer"; static const char kErrMsg[] = "filename must be a string or Buffer";
if (args.Length() < 1) if (args.Length() < 1)

8
src/node_stat_watcher.cc
View File

@ -111,9 +111,15 @@ void StatWatcher::Start(const FunctionCallbackInfo<Value>& args) {
const bool persistent = args[1]->BooleanValue(); const bool persistent = args[1]->BooleanValue();
const uint32_t interval = args[2]->Uint32Value(); const uint32_t interval = args[2]->Uint32Value();
if (!persistent) if (uv_is_active(reinterpret_cast<uv_handle_t*>(wrap->watcher_)))
return;
// Safe, uv_ref/uv_unref are idempotent.
if (persistent)
uv_ref(reinterpret_cast<uv_handle_t*>(wrap->watcher_));
else
uv_unref(reinterpret_cast<uv_handle_t*>(wrap->watcher_)); uv_unref(reinterpret_cast<uv_handle_t*>(wrap->watcher_));
uv_fs_poll_start(wrap->watcher_, Callback, *path, interval); uv_fs_poll_start(wrap->watcher_, Callback, *path, interval);
wrap->ClearWeak(); wrap->ClearWeak();
} }

2
test/parallel/test-fs-watch.js
View File

@ -64,6 +64,8 @@ for (const testCase of cases) {
assert.strictEqual(eventType, 'change'); assert.strictEqual(eventType, 'change');
assert.strictEqual(argFilename, testCase.fileName); assert.strictEqual(argFilename, testCase.fileName);
watcher.start(); // should not crash
// end of test case // end of test case
watcher.close(); watcher.close();
})); }));

9
test/parallel/test-fs-watchfile.js
View File

@ -52,7 +52,8 @@ common.refreshTmpDir();
// time, the callback should be invoked again with proper values in stat object // time, the callback should be invoked again with proper values in stat object
let fileExists = false; let fileExists = false;
fs.watchFile(enoentFile, { interval: 0 }, common.mustCall(function(curr, prev) { const watcher =
fs.watchFile(enoentFile, { interval: 0 }, common.mustCall((curr, prev) => {
if (!fileExists) { if (!fileExists) {
// If the file does not exist, all the fields should be zero and the date // If the file does not exist, all the fields should be zero and the date
// fields should be UNIX EPOCH time // fields should be UNIX EPOCH time
@ -63,8 +64,8 @@ fs.watchFile(enoentFile, { interval: 0 }, common.mustCall(function(curr, prev) {
fs.closeSync(fs.openSync(enoentFile, 'w')); fs.closeSync(fs.openSync(enoentFile, 'w'));
fileExists = true; fileExists = true;
} else { } else {
// If the ino (inode) value is greater than zero, it means that the file is // If the ino (inode) value is greater than zero, it means that the file
// present in the filesystem and it has a valid inode number. // is present in the filesystem and it has a valid inode number.
assert(curr.ino > 0); assert(curr.ino > 0);
// As the file just got created, previous ino value should be lesser than // As the file just got created, previous ino value should be lesser than
// or equal to zero (non-existent file). // or equal to zero (non-existent file).
@ -74,6 +75,8 @@ fs.watchFile(enoentFile, { interval: 0 }, common.mustCall(function(curr, prev) {
} }
}, 2)); }, 2));
watcher.start(); // should not crash
// Watch events should callback with a filename on supported systems. // Watch events should callback with a filename on supported systems.
// Omitting AIX. It works but not reliably. // Omitting AIX. It works but not reliably.
if (common.isLinux || common.isOSX || common.isWindows) { if (common.isLinux || common.isOSX || common.isWindows) {