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

Promise.resolve() does not work correctly inside an inotify callback #67

Closed
redneb opened this issue Sep 4, 2017 · 6 comments
Closed

Comments

@redneb
Copy link

redneb commented Sep 4, 2017

Promise.resolve() returns an already resolved promise and therefore Promise.resolve().then(...) should execute the promise callback immediately. But when I try that from within an inotify callback, the promise callback never gets called. Here's a minimal example:

const Inotify = require("inotify").Inotify;

new Inotify().addWatch({
	path: ".",
	watch_for: Inotify.IN_ALL_EVENTS,
	callback: ev => {
		console.log("this is an inotify callback");
		Promise.resolve().then(() => {
			console.log("done");
		});
	},
});

When I run this with node v8.4.0 and do a touch . in a different terminal, I do not see done in the output.

@c4milo
Copy link
Owner

c4milo commented Sep 5, 2017

@redneb I don't think we have added support for promises yet. Any help is appreciated.

@redneb
Copy link
Author

redneb commented Sep 5, 2017

This issue is not about promises per se, nor I'm asking you to add support for promises to node-inotify. I am just pointing out that while the following piece of code:

console.log("hello");
Promise.resolve().then(() => {
	console.log("done");
});

works as expected when executed by itself (i.e. it produces two lines of output, hello and done)), when executed from within an inotify callback, it does not work as expected. So the behavior changes.

@redneb
Copy link
Author

redneb commented Sep 5, 2017

Here's another complete example that has the same problem and does not use promises at all:

const Inotify = require("inotify").Inotify;

new Inotify().addWatch({
	path: ".",
	watch_for: Inotify.IN_ALL_EVENTS,
	callback: ev => {
		console.log("hello");
		process.nextTick(() => {
			console.log("done");
		});
	},
});

I expect that every time the above program prints hello, it would be (almost) immediately followed by another line of output, done. But it doesn't. It's as if the next tick never happens.

@c4milo
Copy link
Owner

c4milo commented Sep 5, 2017

@redneb thanks for the detailed issue report! I'm looking into it and I will keep you posted here.

@c4milo
Copy link
Owner

c4milo commented Sep 5, 2017

@redneb, alright, I think I know what I need to do to fix this. I will find some time tonight to do it.

We need to change

callback->Call(handle, 1, argv);
to use https://github.com/nodejs/nan/blob/master/doc/callback.md#api_nan_callback

@c4milo c4milo closed this as completed in 9a857cd Sep 10, 2017
@c4milo
Copy link
Owner

c4milo commented Sep 10, 2017

@redneb thanks again, the fix landed on v1.4.2.

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