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

fs: fs.watchFile() should not emit 'stop' event synchronously #8421

Closed
bnoordhuis opened this issue Sep 6, 2016 · 3 comments
Closed

fs: fs.watchFile() should not emit 'stop' event synchronously #8421

bnoordhuis opened this issue Sep 6, 2016 · 3 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@bnoordhuis
Copy link
Member

'use strict';
const assert = require('assert');
const fs = require('fs');

const w = fs.watchFile(__filename, () => {});
w.once('stop', assert.fail);  // Should not trigger.
w.stop();
w.removeListener('stop', assert.fail);

That is bad because w.on('stop', w.stop); w.stop(); will result in a 'maximum call stack size exceeded' error.

@bnoordhuis bnoordhuis added the fs Issues and PRs related to the fs subsystem / file system. label Sep 6, 2016
@claudiorodriguez
Copy link
Contributor

claudiorodriguez commented Sep 6, 2016

Wrapping https://github.com/nodejs/node/blob/master/lib/fs.js#L1466 in a process.nextTick seems to pass all tests, including the one you put here. I'll make a PR if that makes sense

@claudiorodriguez
Copy link
Contributor

@bnoordhuis would the proposed solution work in your case?

@bnoordhuis
Copy link
Member Author

Can you file a pull request? If it's not a proper solution, the reviewer will tell you.

claudiorodriguez added a commit to claudiorodriguez/node that referenced this issue Sep 19, 2016
Emits 'stop' event for fs.watchFile on process.nextTick
to fix 'maximum call stack size exceeded' error when
`stop` is called synchronously after listener is attached.

Fixes: nodejs#8421
jasnell pushed a commit that referenced this issue Sep 29, 2016
Emits 'stop' event for fs.watchFile on process.nextTick
to fix 'maximum call stack size exceeded' error when
`stop` is called synchronously after listener is attached.

PR-URL: #8524
Fixes: #8421
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants