Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chokidar causes app crash if removing nested directories in Node 10 in Windows #730

Closed
srguiwiz opened this issue Jun 11, 2018 · 3 comments

Comments

@srguiwiz
Copy link
Contributor

srguiwiz commented Jun 11, 2018

Using release 2.0.3 here is reproducible code.
Proof is at the end, crashes before getting to the end.
Occurs in Windows only.
Also proves probably should not blame fs.watch.

//

// this is about removing nested directories in Windows when chokidar is watching
// see https://github.com/paulmillr/chokidar/issues/730
//
// the problem is if after
/*
removing now
event, path  unlinkDir test-chokidar\nested\nested2
*/
//it exits the application in a crash, showing
/*
  - node  test-fs-watch.js[8916]: src\fs_event_wrap.cc:90: Assertion `wrap != nullptr' failed.
 1: node::DecodeWrite
 2: node::DecodeWrite
 3: node::Start
 4: v8::internal::interpreter::BytecodeDecoder::Decode
 5: v8::internal::RegExpImpl::Exec
 6: v8::internal::RegExpImpl::Exec
 7: v8::internal::Object::ToInt32
 8: v8::internal::Object::GetProperty
 9: v8::internal::NativesCollection<0>::GetScriptsSource
10: v8::internal::NativesCollection<0>::GetScriptsSource
11: ...
*/

//
// first prove fs.watch doesn't have the problem
//
setTimeout(() => {
  const fs = require("fs");

  const fse = require("fs-extra");

  if (process.platform !== "win32") {
    console.log("this code is meant to help find an issue with chokidar in Windows");
  } else {
    console.log("this code with fs.watch is expected to run fine, error EPERM is ok, doesn't crash");
  }

  fse.ensureDirSync("test-fs-watch/nested/nested2")

  const watchers = [];
  watchers.push(fs.watch("test-fs-watch"));
  watchers.push(fs.watch("test-fs-watch/nested"));
  watchers.push(fs.watch("test-fs-watch/nested/nested2"));

  for (let i = 0; i < watchers.length; i++) {
    let watcher = watchers[i];
    watcher.on("change", (eventType, filename) => console.log(`change ${1}: eventType, filename `, eventType, filename));
    watcher.on("error", (eventType, filename) => console.log(`error ${i}: eventType, filename `, eventType, filename));
  }

  setTimeout(() => {
    console.log("removing now");
    fse.remove("test-fs-watch/nested");
  }, 3000);

  setTimeout(() => {
    console.log("application was still running just fine");
    console.log("");
    //process.exit();
  }, 6000);
},
0 // run this one first
);

//
// then prove fs.chokidar has the problem
//
setTimeout(() => {
  const chokidar = require("chokidar");

  const fse = require("fs-extra");

  if (process.platform !== "win32") {
    console.log("this code is meant to help find an issue with chokidar in Windows");
  } else {
    console.log("this code with chokidar.watch is expected to crash, and only run fine after the issue will have been fixed");
  }

  fse.ensureDirSync("test-chokidar/nested/nested2")

  const watcher = chokidar.watch("test-chokidar");

  watcher.on("all", (event, path) => { console.log("event, path ", event, path); });

  setTimeout(() => {
    console.log("removing now");
    fse.remove("test-chokidar/nested");
  }, 3000);

  setTimeout(() => {
    console.log("application was still running just fine");
    console.log("apparently the issue has been fixed");
    process.exit();
  }, 6000);
},
10000 // run this one second
);

//
@srguiwiz
Copy link
Contributor Author

srguiwiz commented Jun 11, 2018

Progress in investigation. It is a call to watcher.close() that causes the app crash. Proof is same assertion failure if in above listed test of fs.watch after the remove at 3000ms but before the "running just fine" log insert lines

  setTimeout(() => {
    for (let i = watchers.length - 1; i >= 1; i--) {
      let watcher = watchers[i];
      watcher.close();
    }
  }, 5000);

and the same if using

    for (let i = 1; i < watchers.length; i++) {

Apparently this only happens in Node 10, versus problem does not occur in Node 8. Verified in Node 10.1.0 versus Node 8.11.2. As mentioned above, Windows only.

May have to do with Node 10 having a change of architecture, including emitting a close event from an fs.FSWatcher per https://nodejs.org/api/fs.html#fs_class_fs_fswatcher .

@srguiwiz srguiwiz changed the title chokidar forces app crash if removing nested directories in Windows chokidar causes app crash if removing nested directories in Node 10 in Windows Jun 11, 2018
srguiwiz added a commit to srguiwiz/chokidar that referenced this issue Jun 12, 2018
Could not tell whether this is the best possible fix for issue paulmillr#730,
but so far this apparently prevents crashes in some apps.
@srguiwiz
Copy link
Contributor Author

Have submitted a patch here #731. It apparently works for us, but am not hundred percent sure it is ideal.

Also have submitted an issue at Node nodejs/node#21276, short sample code without chokidar.

srguiwiz added a commit to srguiwiz/chokidar that referenced this issue Jun 12, 2018
@srguiwiz
Copy link
Contributor Author

Better patch in #732.

es128 added a commit that referenced this issue Jun 14, 2018
Prevent watcher.close() crashing - fix Issue #730
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants