Merge pull request #127 from theia-ide/mp/linux-event-fix

linux: fix missing events

Author: implausible

binding.gyp

@@ -62,6 +62,9 @@
                 }
             }],
             ["OS=='linux' or OS=='freebsd'", {
+                "defines": [
+                    "NSFW_TEST_SLOW_<!(node -p process.env.NSFW_TEST_SLOW)"
+                ],
                 "sources": [
                     "src/linux/InotifyEventLoop.cpp",
                     "src/linux/InotifyTree.cpp",
@@ -102,7 +105,7 @@
                 ],
                 "libraries": [
                     "-L/usr/local/lib",
-                    "-linotify" 
+                    "-linotify"
                 ]
             }],
         ]

includes/linux/InotifyTree.h

@@ -10,12 +10,20 @@
 #include <vector>
 #include <map>
 #include <unordered_set>
+#include <functional>
+#include <chrono>
+#include <thread>
+
+/**
+ * void EmitCreatedEvent(std::string directory, std::string file);
+ */
+using EmitCreatedEvent = std::function<void(std::string, std::string)>;
 
 class InotifyTree {
 public:
   InotifyTree(int inotifyInstance, std::string path);
 
-  void addDirectory(int wd, std::string name);
+  void addDirectory(int wd, std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
   std::string getError();
   bool getPath(std::string &out, int wd);
   bool hasErrored();
@@ -34,10 +42,11 @@ class InotifyTree {
       InotifyNode *parent,
       std::string directory,
       std::string name,
-      ino_t inodeNumber
+      ino_t inodeNumber,
+      EmitCreatedEvent emitCreatedEvent = nullptr
     );
 
-    void addChild(std::string name);
+    void addChild(std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
     void fixPaths();
     std::string getFullPath();
     std::string getName();
@@ -60,7 +69,8 @@ class InotifyTree {
                                 | IN_MODIFY
                                 | IN_MOVED_FROM
                                 | IN_MOVED_TO
-                                | IN_DELETE_SELF;
+                                | IN_DELETE_SELF
+                                | IN_ONLYDIR;
 
     bool mAlive;
     std::map<std::string, InotifyNode *> *mChildren;

js/scripts/test.js

@@ -0,0 +1,18 @@
+const cp = require('child_process');
+
+if (process.platform !== 'win32' && process.platform !== 'darwin') {
+  // When ran as a npm script we can invoke npm bins such as node-gyp and mocha directly
+  exec('node-gyp rebuild', { env: { ...process.env, NSFW_TEST_SLOW: 1 } });
+  exec('mocha --exit --expose-gc js/spec/index-slow-spec.js');
+  exec('node-gyp rebuild');
+}
+exec('mocha --exit --expose-gc js/spec/index-spec.js');
+
+/**
+ * @param {string} commandline ...
+ * @param {cp.ExecSyncOptions} options ...
+ * @returns {Promise<void>} ...
+ */
+function exec(commandline, options = {}) {
+  cp.execSync(commandline, { stdio: 'inherit', ...options });
+}

js/spec/common.js

@@ -0,0 +1,8 @@
+const path = require('path');
+const util = require('util');
+
+exports.DEBOUNCE = 1000;
+exports.TIMEOUT_PER_STEP = 3000;
+exports.WORKDIR = path.resolve(__dirname, '../../mockfs');
+
+exports.sleep = util.promisify(setTimeout);

js/spec/index-slow-spec.js

@@ -0,0 +1,81 @@
+const assert = require('assert');
+const fse = require('fs-extra');
+const path = require('path');
+
+const { DEBOUNCE, TIMEOUT_PER_STEP, WORKDIR: workDir, sleep } = require('./common');
+
+const nsfw = require('../src');
+
+describe('Node Sentinel File Watcher (slow)', function() {
+  this.timeout(120000);
+
+  assert.ok(nsfw._native.NSFW_TEST_SLOW === true, 'NSFW should be built in slow mode');
+
+  beforeEach(function() {
+    return fse.mkdir(workDir, { recursive: true });
+  });
+
+  afterEach(function() {
+    return fse.remove(workDir);
+  });
+
+  it('can listen for the creation of a deeply nested file', async function () {
+    const folders = 'a_very_deep_tree'.split(''); // ['a', '_', ...]
+    const file = 'a_file.txt';
+
+    // Resolve a promise as soon as all events have been found
+    let done; const promise = new Promise(resolve => {
+      done = resolve;
+    });
+
+    /**
+     * We will remove or pop entries as we find them.
+     * This set should be empty at the end of the test.
+     * @type {Set<string>}
+     */
+    const events = new Set();
+    let directory = workDir; for (const folder of folders) {
+      directory = path.join(directory, folder);
+      events.add(directory);
+    }
+    const filePath = path.join(directory, file);
+    events.add(filePath);
+
+    function findEvent(element) {
+      if (element.action === nsfw.actions.CREATED) {
+        const file = path.join(element.directory, element.file);
+        if (events.has(file)) {
+          events.delete(file);
+          if (events.size === 0) {
+            done();
+          }
+        }
+      }
+    }
+
+    let watch = await nsfw(
+      workDir,
+      events => events.forEach(findEvent),
+      { debounceMS: DEBOUNCE }
+    );
+
+    try {
+      await watch.start();
+      await sleep(TIMEOUT_PER_STEP);
+      await fse.mkdirp(directory);
+      await fse.close(await fse.open(filePath, 'w'));
+      await Promise.race([
+        sleep(folders.length * 500 * 1.5),
+        promise,
+      ]);
+
+      // Make sure that we got the events for each nested directory
+      assert.ok(events.size === 0, `Some files were not detected:\n${
+        Array.from(events, file => `- ${file}`).join('\n')
+      }`);
+    } finally {
+      await watch.stop();
+      watch = null;
+    }
+  });
+});

js/spec/index-spec.js

@@ -2,19 +2,15 @@ const assert = require('assert');
 const exec = require('executive');
 const fse = require('fs-extra');
 const path = require('path');
-const { promisify } = require('util');
 
-const nsfw = require('../src/');
+const { DEBOUNCE, TIMEOUT_PER_STEP, WORKDIR: workDir, sleep } = require('./common');
 
-const DEBOUNCE = 1000;
-const TIMEOUT_PER_STEP = 3000;
-
-const sleep = promisify(setTimeout);
+const nsfw = require('../src');
 
 describe('Node Sentinel File Watcher', function() {
   this.timeout(120000);
 
-  const workDir = path.resolve('./mockfs');
+  assert.ok(typeof nsfw._native.NSFW_TEST_SLOW === 'undefined', 'NSFW should NOT be built in slow mode');
 
   beforeEach(async function() {
     async function makeDir(identifier) {
@@ -438,47 +434,6 @@ describe('Node Sentinel File Watcher', function() {
   });
 
   describe('Recursive', function() {
-    it('can listen for the creation of a deeply nested file', async function() {
-      const paths = ['d', 'e', 'e', 'p', 'f', 'o', 'l', 'd', 'e', 'r'];
-      const file = 'a_file.txt';
-      let foundFileCreateEvent = false;
-
-      function findEvent(element) {
-        if (
-          element.action === nsfw.actions.CREATED &&
-          element.directory === path.join(workDir, ...paths) &&
-          element.file === file
-        ) {
-          foundFileCreateEvent = true;
-        }
-      }
-
-      let watch = await nsfw(
-        workDir,
-        events => events.forEach(findEvent),
-        { debounceMS: DEBOUNCE }
-      );
-
-      try {
-        await watch.start();
-        await sleep(TIMEOUT_PER_STEP);
-        let directory = workDir;
-        for (const dir of paths) {
-          directory = path.join(directory, dir);
-          await fse.mkdir(directory);
-          await sleep(60);
-        }
-        const fd = await fse.open(path.join(directory, file), 'w');
-        await fse.close(fd);
-        await sleep(TIMEOUT_PER_STEP);
-
-        assert.ok(foundFileCreateEvent);
-      } finally {
-        await watch.stop();
-        watch = null;
-      }
-    });
-
     it('can listen for the destruction of a directory and its subtree', async function() {
       const inPath = path.resolve(workDir, 'test4');
       let deletionCount = 0;

js/src/index.js

@@ -102,6 +102,8 @@ nsfw.actions = {
   RENAMED: 3
 };
 
+nsfw._native = NSFW;
+
 if (NSFW.getAllocatedInstanceCount) {
   nsfw.getAllocatedInstanceCount = NSFW.getAllocatedInstanceCount;
 }

package.json

@@ -5,7 +5,7 @@
   "main": "js/src/index.js",
   "scripts": {
     "lint": "eslint js/src js/spec",
-    "test": "yarn lint && mocha --expose-gc js/spec"
+    "test": "yarn lint && node js/scripts/test.js"
   },
   "repository": {
     "type": "git",

src/NSFW.cpp

@@ -339,6 +339,13 @@ Napi::Object NSFW::Init(Napi::Env env, Napi::Object exports) {
     ));
   }
 
+  #ifdef NSFW_TEST_SLOW_1
+    nsfwConstructor.DefineProperty(Napi::PropertyDescriptor::Value(
+      "NSFW_TEST_SLOW",
+      Napi::Boolean::New(env, true)
+    ));
+  #endif
+
   constructor = Napi::Persistent(nsfwConstructor);
   constructor.SuppressDestruct();
 

src/linux/InotifyService.cpp

@@ -98,8 +98,10 @@ void InotifyService::createDirectory(int wd, std::string name) {
     return;
   }
 
-  mTree->addDirectory(wd, name);
   dispatch(CREATED, wd, name);
+  mTree->addDirectory(wd, name, [this](std::string directory, std::string file) {
+    mQueue->enqueue(CREATED, directory, file);
+  });
 }
 
 void InotifyService::removeDirectory(int wd) {