-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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(napi): Don't run microtasks in napi_resolve_deferred #25246
fix(napi): Don't run microtasks in napi_resolve_deferred #25246
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be lovely to have a test, but I understand it's really hard. Nice find
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes an incredibly obscure bug that causes parcel's file watcher to not get any file update notifications on macOS. The issue was that the native addon was calling `napi_resolve_deferred`, but when we resolved the promise, v8 was running microtasks automatically. That executed JS, which called back into the native addon and broke the addon's assumption that the call wouldn't be reentrant.
Fixes an incredibly obscure bug that causes parcel's file watcher to not get any file update notifications on macOS.
The issue was that the native addon was calling
napi_resolve_deferred
, but when we resolved the promise, v8 was running microtasks automatically. That executed JS, which called back into the native addon and broke the addon's assumption that the call wouldn't be reentrant.