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

Fix for a race condition on Linux #1228

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ const getAbsolutePath = (path, cwd) => {

const undef = (opts, key) => opts[key] === undefined;

const NODE_MAJOR_VERSION = Number(process.versions.node.split('.')[0]);

/**
* Directory entry.
* @property {Path} path
Expand Down Expand Up @@ -950,6 +952,17 @@ _readdirp(root, opts) {
stream = undefined;
}
});

if (NODE_MAJOR_VERSION < 10) {
// On Node 8 Readable.push() throws when the stream is destroyed
// even though the documentation says it should be silently ignored.
const superPush = stream.push.bind(stream);
stream.push = function (...args) {
if (this.destroyed) return false;
return superPush(...args);
}
}

return stream;
}

Expand Down
5 changes: 5 additions & 0 deletions lib/nodefs-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,11 @@ async _handleDir(dir, stats, initialAdd, depth, target, wh, realpath) {

this._handleRead(dirPath, false, wh, target, dir, depth, throttler);
});

// schedule a rescan of the directory in case something
// appeared while we were setting up the watch
if (!(this.fsw.usePolling || target || this.fsw.closed))
this._handleRead(dir, false, wh, target, dir, depth, throttler);
}
return closer;
}
Expand Down
28 changes: 23 additions & 5 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,29 @@ const runTests = (baseopts) => {
expect(spy.args[0][1]).to.be.ok; // stats
rawSpy.should.have.been.called;
});
if (!(baseopts.usePolling || baseopts.useFsEvents)) context('when a race happens', () => {
beforeEach(() => {
const stub = sinon.stub(fs, 'watch');
stub.callsFake((filename, ...rest) => {
// pretend a sneaky process creates a file just as we're about to set up the watch
if (filename.endsWith('subdir')) fs.writeFileSync(getFixturePath('subdir/add.txt'), dateNow());
return stub.wrappedMethod(filename, ...rest);
});
});
afterEach(() => fs.watch.restore());
it('should notice when a file appears in a new directory anyway', async () => {
const testDir = getFixturePath('subdir');
const testPath = getFixturePath('subdir/add.txt');
const spy = await aspy(watcher, EV_ADD);
spy.should.not.have.been.called;
await fs_mkdir(testDir, PERM_ARR);
await waitFor([spy]);
spy.should.have.been.calledOnce;
spy.should.have.been.calledWith(testPath);
expect(spy.args[0][1]).to.be.ok; // stats
rawSpy.should.have.been.called;
});
});
it('should watch removed and re-added directories', async () => {
const unlinkSpy = sinon.spy(function unlinkSpy(){});
const addSpy = sinon.spy(function addSpy(){});
Expand Down Expand Up @@ -2192,8 +2215,6 @@ const runTests = (baseopts) => {
await waitForWatcher(watcher);
await delay(300);
await write(getFixturePath('add.txt'), 'hello');
await delay(300);
await fs_unlink(getFixturePath('add.txt'));
})();
});
});
Expand All @@ -2216,9 +2237,6 @@ const runTests = (baseopts) => {
await watcher1.close();
// Write a new file into the fixtures to test the EV_ADD event
await write(getFixturePath('add.txt'), 'hello');
// Ensures EV_ADD is called. Immediately removing the file causes it to be skipped
await delay(200);
await fs_unlink(getFixturePath('add.txt'));
})()
})
});
Expand Down