-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
timers: run nextTicks + MTQ between timers / immediates #22316
timers: run nextTicks + MTQ between timers / immediates #22316
Conversation
Ugh, how do I stop those? |
CITGM (I think I did this right): https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/1506/ |
@Fishrock123 I cancelled the Travis CI |
So, I think this might need to be slightly more complicated to account for proper error handling but I'll need to confirm (if next tick queue throws). Will review and test in more detail before tomorrow morning. |
Hmm, CITGM builds seem to have failed? @nodejs/build |
Ok well, this patch allows the tests to run but also causes many async_hooks segfaults: diff --git a/lib/timers.js b/lib/timers.js
index 6e473456b9..090ae60ff4 100644
--- a/lib/timers.js
+++ b/lib/timers.js
@@ -613,7 +613,11 @@ function processImmediate() {
emitAfter(asyncId);
- runNextTicks();
+ try {
+ runNextTicks();
+ } catch (e) {
+
+ }
immediate = immediate._idleNext;
} [04:53|% 100|+ 2356|- 9]: Done
|
@Fishrock123 This happens on architectures which are only supported by some versions of Node.js, but not the one that the CITGM job is testing (ex: Ubuntu 12.04). This is being tracked at nodejs/build#1430 |
Superseded by #22842 |
Refs: #22257
This is semver-major because it changes significant timing guarentees. The tests do not pass because of the changes to Immediates.
Do not run the core tests, they will stall!
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes