Skip to content

Commit 6eadcd9

Browse files
committed
emit CREATED event for new directories
Most clients expect every new file from a new tree being added in the watched directory to be reported. This is currently not the case if the tree appears too quickly on disk. This commit propagates a callback down the recursive tree building to make sure that all files added are being reported as created to clients. I also added an environment variable check for use when testing: `NSFW_TEST_SLOW`. When set to `1` nsfw will artificially sleep before crawling folders on the fs. This is to simulate the case when nsfw is late to the party and sub-folders were created before we had time to allocate inotify watchers. In such a case, we need to manually emit CREATED events for new files and folders discovered by scandir. Signed-off-by: Paul Maréchal <[email protected]>
1 parent 928d9e1 commit 6eadcd9

File tree

8 files changed

+159
-15
lines changed

8 files changed

+159
-15
lines changed

binding.gyp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@
6262
}
6363
}],
6464
["OS=='linux' or OS=='freebsd'", {
65+
"defines": [
66+
"NSFW_TEST_SLOW_<!(node -p process.env.NSFW_TEST_SLOW)"
67+
],
6568
"sources": [
6669
"src/linux/InotifyEventLoop.cpp",
6770
"src/linux/InotifyTree.cpp",
@@ -102,7 +105,7 @@
102105
],
103106
"libraries": [
104107
"-L/usr/local/lib",
105-
"-linotify"
108+
"-linotify"
106109
]
107110
}],
108111
]

includes/linux/InotifyTree.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,20 @@
1010
#include <vector>
1111
#include <map>
1212
#include <unordered_set>
13+
#include <functional>
14+
#include <chrono>
15+
#include <thread>
16+
17+
/**
18+
* void EmitCreatedEvent(std::string directory, std::string file);
19+
*/
20+
using EmitCreatedEvent = std::function<void(std::string, std::string)>;
1321

1422
class InotifyTree {
1523
public:
1624
InotifyTree(int inotifyInstance, std::string path);
1725

18-
void addDirectory(int wd, std::string name);
26+
void addDirectory(int wd, std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
1927
std::string getError();
2028
bool getPath(std::string &out, int wd);
2129
bool hasErrored();
@@ -34,10 +42,11 @@ class InotifyTree {
3442
InotifyNode *parent,
3543
std::string directory,
3644
std::string name,
37-
ino_t inodeNumber
45+
ino_t inodeNumber,
46+
EmitCreatedEvent emitCreatedEvent = nullptr
3847
);
3948

40-
void addChild(std::string name);
49+
void addChild(std::string name, EmitCreatedEvent emitCreatedEvent = nullptr);
4150
void fixPaths();
4251
std::string getFullPath();
4352
std::string getName();

js/scripts/test.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
const cp = require('child_process');
2+
const mochaBin = require.resolve('mocha/bin/mocha');
3+
const stdio = 'inherit';
4+
5+
if (process.platform !== 'win32' && process.platform !== 'darwin') {
6+
cp.execSync('yarn rebuild-slow', { stdio });
7+
spawnSync(process.argv0, [mochaBin, '--expose-gc', 'js/spec/index-slow-spec.js'], { stdio });
8+
cp.execSync('yarn rebuild', { stdio });
9+
}
10+
spawnSync(process.argv0, [mochaBin, '--expose-gc', 'js/spec/index-spec.js'], { stdio });
11+
12+
function spawnSync(...args) {
13+
const { status } = cp.spawnSync(...args);
14+
if (status !== 0) {
15+
process.exit(status);
16+
}
17+
}

js/spec/index-slow-spec.js

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
const assert = require('assert');
2+
const fse = require('fs-extra');
3+
const path = require('path');
4+
const { promisify } = require('util');
5+
6+
const nsfw = require('../src');
7+
8+
const DEBOUNCE = 1000;
9+
const TIMEOUT_PER_STEP = 3000;
10+
11+
const sleep = promisify(setTimeout);
12+
13+
describe('Node Sentinel File Watcher (slow)', function() {
14+
this.timeout(120000);
15+
16+
const workDir = path.resolve('./mockfs');
17+
18+
beforeEach(async function() {
19+
await fse.mkdir(workDir);
20+
});
21+
22+
afterEach(function() {
23+
return fse.remove(workDir);
24+
});
25+
26+
describe('Recursive', function() {
27+
it('can listen for the creation of a deeply nested file', async function() {
28+
const folders = 'a_very_deep_tree'.split(''); // ['a', '_', ...]
29+
const file = 'a_file.txt';
30+
31+
// Resolve a promise as soon as all events have been found
32+
let done; const promise = new Promise(resolve => {
33+
done = resolve;
34+
});
35+
36+
/**
37+
* We will remove or pop entries as we find them.
38+
* This set should be empty at the end of the test.
39+
* @type {Set<string>}
40+
*/
41+
const events = new Set();
42+
let directory = workDir; for (const folder of folders) {
43+
directory = path.join(directory, folder);
44+
events.add(directory);
45+
}
46+
events.add(path.join(directory, file));
47+
48+
// The modified build should emit an artificial event named NSFW_TEST_SLOW/1:
49+
events.add('NSFW_TEST_SLOW/1');
50+
51+
function findEvent(element) {
52+
if (element.action === nsfw.actions.CREATED) {
53+
const file = path.join(element.directory, element.file);
54+
if (events.has(file)) {
55+
events.delete(file);
56+
}
57+
if (events.size === 0) {
58+
done();
59+
}
60+
}
61+
}
62+
63+
let watch = await nsfw(
64+
workDir,
65+
events => events.forEach(findEvent),
66+
{ debounceMS: DEBOUNCE }
67+
);
68+
69+
try {
70+
await watch.start();
71+
await sleep(TIMEOUT_PER_STEP);
72+
await fse.mkdirp(directory);
73+
await fse.close(await fse.open(path.join(directory, file), 'w'));
74+
await Promise.race([
75+
sleep(folders.length * 500 * 1.5),
76+
promise,
77+
]);
78+
79+
// Make sure that we got the events for each nested directory
80+
assert.ok(events.size === 0, `Some files were not detected:\n${
81+
Array.from(events, file => `- ${file}`).join('\n')
82+
}`);
83+
} finally {
84+
await watch.stop();
85+
watch = null;
86+
}
87+
});
88+
});
89+
});

js/spec/index-spec.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -442,6 +442,7 @@ describe('Node Sentinel File Watcher', function() {
442442
const folders = 'a_very_deep_tree'.split(''); // ['a', '_', ...]
443443
const file = 'a_file.txt';
444444

445+
/** @type {Map<string, { file: string, found: boolean }>} */
445446
const events = new Map();
446447
let directory = workDir; for (const folder of folders) {
447448
events.set(directory, { file: folder, found: false });
@@ -459,6 +460,10 @@ describe('Node Sentinel File Watcher', function() {
459460
}
460461
}
461462

463+
// Force nsfw to sleep and hence miss creation events.
464+
// This makes sure we test the right mechanism that should manually
465+
// fire creation events when nested folders are added at once.
466+
process.env['NSFW_TEST_INOTIFYNODE_SLEEP'] = '1';
462467
let watch = await nsfw(
463468
workDir,
464469
events => events.forEach(findEvent),
@@ -470,17 +475,18 @@ describe('Node Sentinel File Watcher', function() {
470475
await sleep(TIMEOUT_PER_STEP);
471476
await fse.mkdirp(directory);
472477
await fse.close(await fse.open(path.join(directory, file), 'w'));
473-
await sleep(TIMEOUT_PER_STEP);
478+
await sleep(folders.length * 200 * 1.5);
474479

480+
// Make sure that we got the events for each nested directory
475481
for (const [directory, item] of events.entries()) {
476482
assert.ok(item.found, `${path.join(directory, item.file)} wasn't found`);
477483
}
478484
} finally {
485+
delete process.env['NSFW_TEST_INOTIFYNODE_SLEEP'];
479486
await watch.stop();
480487
watch = null;
481488
}
482489
});
483-
484490
it('can listen for the destruction of a directory and its subtree', async function() {
485491
const inPath = path.resolve(workDir, 'test4');
486492
let deletionCount = 0;

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
"main": "js/src/index.js",
66
"scripts": {
77
"lint": "eslint js/src js/spec",
8-
"test": "yarn lint && mocha --expose-gc js/spec"
8+
"rebuild": "node-gyp rebuild",
9+
"rebuild-slow": "NSFW_TEST_SLOW=1 node-gyp rebuild",
10+
"test": "yarn lint && node --expose-gc js/scripts/test.js"
911
},
1012
"repository": {
1113
"type": "git",

src/linux/InotifyService.cpp

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ InotifyService::InotifyService(std::shared_ptr<EventQueue> queue, std::string pa
1616
mTree = NULL;
1717
mEventLoop = NULL;
1818
} else {
19+
#ifdef NSFW_TEST_SLOW_1
20+
// Emit a fake event to notify clients that this is a test build
21+
mQueue->enqueue(CREATED, "NSFW_TEST_SLOW", "1");
22+
#endif
1923
mEventLoop = new InotifyEventLoop(
2024
mInotifyInstance,
2125
this
@@ -98,8 +102,10 @@ void InotifyService::createDirectory(int wd, std::string name) {
98102
return;
99103
}
100104

101-
mTree->addDirectory(wd, name);
102105
dispatch(CREATED, wd, name);
106+
mTree->addDirectory(wd, name, [this](std::string directory, std::string file) {
107+
mQueue->enqueue(CREATED, directory, file);
108+
});
103109
}
104110

105111
void InotifyService::removeDirectory(int wd) {

src/linux/InotifyTree.cpp

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
11
#include "../../includes/linux/InotifyTree.h"
2+
#include <cstdio>
3+
#include <sys/stat.h>
24
/**
35
* InotifyTree ---------------------------------------------------------------------------------------------------------
46
*/
57
InotifyTree::InotifyTree(int inotifyInstance, std::string path):
68
mError(""),
79
mInotifyInstance(inotifyInstance) {
810
mInotifyNodeByWatchDescriptor = new std::map<int, InotifyNode *>;
9-
1011
std::string directory;
1112
std::string watchName;
1213
if (path.length() == 1 && path[0] == '/') {
@@ -41,14 +42,14 @@ InotifyTree::InotifyTree(int inotifyInstance, std::string path):
4142
}
4243
}
4344

44-
void InotifyTree::addDirectory(int wd, std::string name) {
45+
void InotifyTree::addDirectory(int wd, std::string name, EmitCreatedEvent emitCreatedEvent) {
4546
auto nodeIterator = mInotifyNodeByWatchDescriptor->find(wd);
4647
if (nodeIterator == mInotifyNodeByWatchDescriptor->end()) {
4748
return;
4849
}
4950

5051
InotifyNode *node = nodeIterator->second;
51-
node->addChild(name);
52+
node->addChild(name, emitCreatedEvent);
5253
}
5354

5455
void InotifyTree::addNodeReferenceByWD(int wd, InotifyNode *node) {
@@ -159,7 +160,8 @@ InotifyTree::InotifyNode::InotifyNode(
159160
InotifyNode *parent,
160161
std::string directory,
161162
std::string name,
162-
ino_t inodeNumber
163+
ino_t inodeNumber,
164+
EmitCreatedEvent emitCreatedEvent
163165
):
164166
mDirectory(directory),
165167
mInodeNumber(inodeNumber),
@@ -171,6 +173,10 @@ InotifyTree::InotifyNode::InotifyNode(
171173
mFullPath = createFullPath(mDirectory, mName);
172174
mWatchDescriptorInitialized = false;
173175

176+
#ifdef NSFW_TEST_SLOW_1
177+
std::this_thread::sleep_for(std::chrono::milliseconds(500));
178+
#endif
179+
174180
if (!inotifyInit()) {
175181
return;
176182
}
@@ -200,6 +206,10 @@ InotifyTree::InotifyNode::InotifyNode(
200206
continue;
201207
}
202208

209+
if (emitCreatedEvent) {
210+
emitCreatedEvent(mFullPath, fileName);
211+
}
212+
203213
std::string filePath = createFullPath(mFullPath, fileName);
204214

205215
struct stat file;
@@ -218,7 +228,8 @@ InotifyTree::InotifyNode::InotifyNode(
218228
this,
219229
mFullPath,
220230
fileName,
221-
file.st_ino
231+
file.st_ino,
232+
emitCreatedEvent
222233
);
223234

224235
if (child->isAlive()) {
@@ -250,7 +261,7 @@ InotifyTree::InotifyNode::~InotifyNode() {
250261
delete mChildren;
251262
}
252263

253-
void InotifyTree::InotifyNode::addChild(std::string name) {
264+
void InotifyTree::InotifyNode::addChild(std::string name, EmitCreatedEvent emitCreatedEvent) {
254265
struct stat file;
255266

256267
if (stat(createFullPath(mFullPath, name).c_str(), &file) >= 0 && mTree->addInode(file.st_ino)) {
@@ -260,7 +271,8 @@ void InotifyTree::InotifyNode::addChild(std::string name) {
260271
this,
261272
mFullPath,
262273
name,
263-
file.st_ino
274+
file.st_ino,
275+
emitCreatedEvent
264276
);
265277

266278
if (child->isAlive()) {

0 commit comments

Comments
 (0)