[v9.x-backport] Backport #17198, #17738, #17841, #17736, #17881, #17879 and #18139 #19006

apapirovski · 2018-02-26T15:53:09Z

As per title, backports a whole bunch of inter-dependent PRs.

Note that #17198 used to be marked as semver-major but several TSC members have approved it being downgraded as it's non-breaking and is a dependency for several subsequent changes.

There is at least one semver-minor PR in here so this can't land until 9.7.0 at the earliest.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

Share `process` through the module wrapper rather than relying on nobody messing with `global.process`. PR-URL: nodejs#17198 Fixes: nodejs#6802 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Do not share unnecessary information about nextTick state between JS & C++, instead only track whether a nextTick is scheduled or not. Turn nextTickQueue into an Object instead of a class since multiple instances are never created. Other assorted refinements and refactoring. PR-URL: nodejs#17738 Reviewed-By: Anna Henningsen <anna@addaleax.net>

When a process encounters a _fatalException that is caught, it should schedule execution of nextTicks but not in an arbitrary place of the next Immediates queue. Instead, add a no-op function to the queue that will ensure processImmediate runs, which will then ensure that nextTicks are processed at the end. PR-URL: nodejs#17841 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

Make setImmediate() immune to `process` global tampering by removing the dependency on the `process._immediateCallback` property. PR-URL: nodejs#17736 Fixes: nodejs#17681 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: nodejs#17881 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

If an error is encountered during the processing of Immediates, schedule the remaining queue to finish after all error handling code runs (if the process is still alive to do so). The new changes make the Immediates error handling behaviour entirely deterministic and predictable, as the full queue will be flushed on each Immediates cycle, regardless of whether an error is encountered or not. Currently this processing is scheduled for nextTick which can yield unpredictable results as the nextTick might happen as early as close callbacks phase or as late as after the next event loop turns Immediates all fully processed. The latter can result in two full cycles of Immediates processing during one even loop turn. The current implementation also doesn't differentiate between Immediates scheduled for the current queue run or the next one, so Immediates that were scheduled for the next turn of the event loop, will process alongside the ones that were scheduled for the current turn. PR-URL: nodejs#17879 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

Refactor Immediates handling to allow for them to be unrefed, similar to setTimeout, but without extra handles. Document the new `immediate.ref()` and `immediate.unref()` methods. Add SetImmediateUnref on the C++ side. PR-URL: nodejs#18139 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

apapirovski · 2018-02-26T15:53:46Z

CI: https://ci.nodejs.org/job/node-test-pull-request/13374/

addaleax

Nice, thanks!

Share `process` through the module wrapper rather than relying on nobody messing with `global.process`. Backport-PR-URL: #19006 PR-URL: #17198 Fixes: #6802 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>

Do not share unnecessary information about nextTick state between JS & C++, instead only track whether a nextTick is scheduled or not. Turn nextTickQueue into an Object instead of a class since multiple instances are never created. Other assorted refinements and refactoring. Backport-PR-URL: #19006 PR-URL: #17738 Reviewed-By: Anna Henningsen <anna@addaleax.net>

When a process encounters a _fatalException that is caught, it should schedule execution of nextTicks but not in an arbitrary place of the next Immediates queue. Instead, add a no-op function to the queue that will ensure processImmediate runs, which will then ensure that nextTicks are processed at the end. Backport-PR-URL: #19006 PR-URL: #17841 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

Make setImmediate() immune to `process` global tampering by removing the dependency on the `process._immediateCallback` property. Backport-PR-URL: #19006 PR-URL: #17736 Fixes: #17681 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

Backport-PR-URL: #19006 PR-URL: #17881 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Tobias Nießen <tniessen@tnie.de>

If an error is encountered during the processing of Immediates, schedule the remaining queue to finish after all error handling code runs (if the process is still alive to do so). The new changes make the Immediates error handling behaviour entirely deterministic and predictable, as the full queue will be flushed on each Immediates cycle, regardless of whether an error is encountered or not. Currently this processing is scheduled for nextTick which can yield unpredictable results as the nextTick might happen as early as close callbacks phase or as late as after the next event loop turns Immediates all fully processed. The latter can result in two full cycles of Immediates processing during one even loop turn. The current implementation also doesn't differentiate between Immediates scheduled for the current queue run or the next one, so Immediates that were scheduled for the next turn of the event loop, will process alongside the ones that were scheduled for the current turn. Backport-PR-URL: #19006 PR-URL: #17879 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

Refactor Immediates handling to allow for them to be unrefed, similar to setTimeout, but without extra handles. Document the new `immediate.ref()` and `immediate.unref()` methods. Add SetImmediateUnref on the C++ side. Backport-PR-URL: #19006 PR-URL: #18139 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>

addaleax · 2018-02-26T18:00:01Z

Thanks, landed on v9.x-staging!

landed in 743cf33...f2dd17b