lib: add primordials.SafeArrayIterator #36532

aduh95 · 2020-12-15T17:21:16Z

Make Node.js module loading resilient to prototype tempering from user-mode.

Checklist

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

lib/internal/buffer.js

mscdex

There's quite a few performance regressions here. I expect these will show up for other subsystem benchmarks as well due to the code that is being changed here, but here are the Buffer creation benchmark results for example:

                                                                     confidence improvement accuracy (*)   (**)   (***)
buffers/buffer-creation.js n=600000 len=1024 type='fast-alloc'              ***    -16.77 %       ±3.11% ±4.15%  ±5.42%
buffers/buffer-creation.js n=600000 len=1024 type='fast-alloc-fill'         ***    -19.31 %       ±4.12% ±5.50%  ±7.18%
buffers/buffer-creation.js n=600000 len=1024 type='fast-allocUnsafe'        ***    -60.95 %       ±2.93% ±3.91%  ±5.12%
buffers/buffer-creation.js n=600000 len=1024 type='slow-allocUnsafe'        ***    -25.14 %       ±3.70% ±4.93%  ±6.43%
buffers/buffer-creation.js n=600000 len=10 type='fast-alloc'                ***    -84.48 %       ±4.53% ±6.09%  ±8.07%
buffers/buffer-creation.js n=600000 len=10 type='fast-alloc-fill'           ***    -80.82 %       ±4.60% ±6.18%  ±8.16%
buffers/buffer-creation.js n=600000 len=10 type='fast-allocUnsafe'          ***    -87.77 %       ±7.33% ±9.88% ±13.12%
buffers/buffer-creation.js n=600000 len=10 type='slow-allocUnsafe'          ***    -83.53 %       ±6.69% ±9.01% ±11.95%
buffers/buffer-creation.js n=600000 len=4096 type='fast-alloc'              ***    -16.15 %       ±2.99% ±3.98%  ±5.18%
buffers/buffer-creation.js n=600000 len=4096 type='fast-alloc-fill'         ***    -18.88 %       ±3.98% ±5.31%  ±6.92%
buffers/buffer-creation.js n=600000 len=4096 type='fast-allocUnsafe'        ***    -28.35 %       ±3.52% ±4.70%  ±6.15%
buffers/buffer-creation.js n=600000 len=4096 type='slow-allocUnsafe'        ***    -26.75 %       ±3.74% ±4.98%  ±6.49%
buffers/buffer-creation.js n=600000 len=8192 type='fast-alloc'              ***    -11.73 %       ±3.51% ±4.67%  ±6.08%
buffers/buffer-creation.js n=600000 len=8192 type='fast-alloc-fill'         ***    -14.17 %       ±3.54% ±4.72%  ±6.15%
buffers/buffer-creation.js n=600000 len=8192 type='fast-allocUnsafe'        ***    -27.15 %       ±3.93% ±5.25%  ±6.87%
buffers/buffer-creation.js n=600000 len=8192 type='slow-allocUnsafe'        ***    -26.28 %       ±3.97% ±5.32%  ±6.98%

lib/internal/buffer.js

aduh95 · 2020-12-21T13:09:52Z

@mscdex Please check out the last benchmark results, they seem much better now.

                                                                      confidence improvement accuracy (*)    (**)   (***)
 buffers/buffer-creation.js n=600000 len=1024 type='fast-alloc'                      -0.73 %       ±4.90%  ±6.52%  ±8.50%
 buffers/buffer-creation.js n=600000 len=1024 type='fast-alloc-fill'                 -2.15 %       ±5.16%  ±6.86%  ±8.93%
 buffers/buffer-creation.js n=600000 len=1024 type='fast-allocUnsafe'                -3.03 %       ±4.77%  ±6.35%  ±8.28%
 buffers/buffer-creation.js n=600000 len=1024 type='slow-allocUnsafe'                -1.76 %       ±4.70%  ±6.26%  ±8.15%
 buffers/buffer-creation.js n=600000 len=10 type='fast-alloc'                         1.64 %       ±9.06% ±12.06% ±15.71%
 buffers/buffer-creation.js n=600000 len=10 type='fast-alloc-fill'                    0.59 %       ±7.80% ±10.39% ±13.56%
 buffers/buffer-creation.js n=600000 len=10 type='fast-allocUnsafe'                   0.18 %       ±5.42%  ±7.22%  ±9.39%
 buffers/buffer-creation.js n=600000 len=10 type='slow-allocUnsafe'                   0.93 %      ±10.22% ±13.60% ±17.70%
 buffers/buffer-creation.js n=600000 len=4096 type='fast-alloc'                       3.30 %       ±6.85%  ±9.13% ±11.90%
 buffers/buffer-creation.js n=600000 len=4096 type='fast-alloc-fill'           *      4.63 %       ±3.90%  ±5.19%  ±6.77%
 buffers/buffer-creation.js n=600000 len=4096 type='fast-allocUnsafe'                 0.38 %       ±5.08%  ±6.78%  ±8.86%
 buffers/buffer-creation.js n=600000 len=4096 type='slow-allocUnsafe'                 2.40 %       ±5.04%  ±6.70%  ±8.73%
 buffers/buffer-creation.js n=600000 len=8192 type='fast-alloc'                       0.07 %       ±3.53%  ±4.71%  ±6.13%
 buffers/buffer-creation.js n=600000 len=8192 type='fast-alloc-fill'                 -0.09 %       ±3.59%  ±4.77%  ±6.21%
 buffers/buffer-creation.js n=600000 len=8192 type='fast-allocUnsafe'                -3.26 %       ±5.65%  ±7.52%  ±9.81%
 buffers/buffer-creation.js n=600000 len=8192 type='slow-allocUnsafe'                 0.32 %       ±3.83%  ±5.10%  ±6.64%

                                                                                   confidence improvement accuracy (*)    (**)   (***)
 module/module-loader-circular.js n=10000                                                          1.21 %       ±3.68%  ±4.90%  ±6.38%
 module/module-loader-deep.js cache='false' files=1000 ext=''                                     -0.08 %       ±4.05%  ±5.39%  ±7.02%
 module/module-loader-deep.js cache='false' files=1000 ext='.js'                                  -1.50 %       ±4.39%  ±5.84%  ±7.60%
 module/module-loader-deep.js cache='true' files=1000 ext=''                                      -1.71 %       ±7.02%  ±9.34% ±12.18%
 module/module-loader-deep.js cache='true' files=1000 ext='.js'                                    3.86 %       ±8.70% ±11.58% ±15.07%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/'                         4.41 %       ±4.61%  ±6.15%  ±8.04%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name=''                         -1.81 %       ±3.66%  ±4.86%  ±6.33%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/index.js'                 4.72 %       ±5.11%  ±6.83%  ±8.95%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/'                        -1.44 %       ±4.39%  ±5.86%  ±7.67%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name=''                          3.64 %       ±3.95%  ±5.25%  ±6.84%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/index.js'                 0.99 %       ±3.79%  ±5.05%  ±6.58%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name='/'                            3.18 %       ±6.40%  ±8.52% ±11.10%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name=''                            -3.88 %       ±5.93%  ±7.89% ±10.28%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name='/index.js'                   -0.04 %       ±5.71%  ±7.60%  ±9.89%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name='/'                            1.19 %       ±4.96%  ±6.60%  ±8.59%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name=''                             0.37 %       ±4.78%  ±6.36%  ±8.28%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name='/index.js'                    3.20 %       ±7.49%  ±9.98% ±13.03%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name='/'                          1.25 %       ±1.72%  ±2.29%  ±2.98%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name=''                          -0.22 %       ±1.41%  ±1.88%  ±2.45%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name='/index.js'          **      2.90 %       ±2.10%  ±2.81%  ±3.72%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name='/'                          2.38 %       ±3.37%  ±4.53%  ±5.99%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name=''                          -0.82 %       ±1.80%  ±2.39%  ±3.12%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name='/index.js'                  1.17 %       ±1.97%  ±2.62%  ±3.41%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name='/'                      *     10.14 %       ±8.63% ±11.50% ±15.00%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name=''                       *      5.30 %       ±4.04%  ±5.44%  ±7.21%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name='/index.js'                    -2.71 %       ±6.27%  ±8.43% ±11.15%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name='/'                             0.40 %       ±9.24% ±12.29% ±16.00%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name=''                             -0.07 %       ±3.66%  ±4.87%  ±6.35%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name='/index.js'              *      8.14 %       ±6.96%  ±9.28% ±12.11%

aduh95 · 2020-12-22T19:22:29Z

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/789/

                                                                                   confidence improvement accuracy (*)    (**)   (***)
 module/module-loader-circular.js n=10000                                                         -0.20 %       ±3.34%  ±4.45%  ±5.79%
 module/module-loader-deep.js cache='false' files=1000 ext=''                                     -3.41 %       ±3.74%  ±4.99%  ±6.51%
 module/module-loader-deep.js cache='false' files=1000 ext='.js'                                  -1.58 %       ±4.00%  ±5.32%  ±6.93%
 module/module-loader-deep.js cache='true' files=1000 ext=''                                      -3.46 %       ±8.03% ±10.70% ±13.93%
 module/module-loader-deep.js cache='true' files=1000 ext='.js'                                    1.59 %       ±8.91% ±11.85% ±15.43%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/'                         2.77 %       ±3.59%  ±4.78%  ±6.23%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name=''                         -2.25 %       ±4.18%  ±5.60%  ±7.35%
 module/module-loader.js cache='false' n=1000 files=500 dir='abs' name='/index.js'          *     -3.92 %       ±3.89%  ±5.18%  ±6.75%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/'                        -1.54 %       ±3.69%  ±4.90%  ±6.38%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name=''                          1.71 %       ±2.30%  ±3.06%  ±3.99%
 module/module-loader.js cache='false' n=1000 files=500 dir='rel' name='/index.js'                 2.28 %       ±5.26%  ±7.02%  ±9.16%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name='/'                            0.49 %       ±6.30%  ±8.39% ±10.92%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name=''                            -0.70 %       ±6.01%  ±8.00% ±10.42%
 module/module-loader.js cache='false' n=1 files=500 dir='abs' name='/index.js'                    3.36 %       ±3.93%  ±5.23%  ±6.80%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name='/'                           -1.52 %       ±4.89%  ±6.51%  ±8.47%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name=''                             2.01 %       ±5.35%  ±7.12%  ±9.27%
 module/module-loader.js cache='false' n=1 files=500 dir='rel' name='/index.js'                    0.69 %       ±4.13%  ±5.50%  ±7.16%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name='/'                          0.41 %       ±1.13%  ±1.50%  ±1.96%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name=''                          -0.73 %       ±1.87%  ±2.49%  ±3.24%
 module/module-loader.js cache='true' n=1000 files=500 dir='abs' name='/index.js'                  1.14 %       ±1.63%  ±2.17%  ±2.85%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name='/'                         -0.41 %       ±2.13%  ±2.84%  ±3.69%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name=''                           1.32 %       ±4.85%  ±6.52%  ±8.62%
 module/module-loader.js cache='true' n=1000 files=500 dir='rel' name='/index.js'                 -0.79 %       ±1.19%  ±1.59%  ±2.07%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name='/'                             2.01 %       ±4.19%  ±5.62%  ±7.40%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name=''                             -3.18 %       ±5.20%  ±6.92%  ±9.01%
 module/module-loader.js cache='true' n=1 files=500 dir='abs' name='/index.js'                     0.00 %       ±4.46%  ±5.99%  ±7.91%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name='/'                            -0.12 %       ±0.66%  ±0.87%  ±1.14%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name=''                             -0.15 %       ±2.03%  ±2.71%  ±3.54%
 module/module-loader.js cache='true' n=1 files=500 dir='rel' name='/index.js'                    -1.22 %       ±4.25%  ±5.66%  ±7.37%

Trott

LGTM once unblocked (which presumably will make the failing tests pass)

Trott · 2020-12-23T14:05:18Z

@nodejs/modules

Trott

LGTM once unblocked (which presumably will make the failing tests pass)

Using an explicit constructor is necessary to avoid relying on `Array.prototype[Symbol.iterator]` and `%ArrayIteratorPrototype%.next`, which can be mutated by users. PR-URL: #36587 Refs: #36428 Refs: #36532 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

nodejs-github-bot · 2020-12-25T19:32:19Z

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

aduh95 · 2020-12-26T10:16:57Z

Benchmark CI (events): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/803/console
Benchmark CI (streams): https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/804/console

Those don't show any perf regression either.

                                                           confidence improvement accuracy (*)   (**)  (***)
 events/ee-add-remove.js n=1000000                                        -1.13 %       ±2.86% ±3.81% ±4.99%
 events/ee-emit.js listeners=10 argc=0 n=2000000                          -0.38 %       ±1.44% ±1.92% ±2.50%
 events/ee-emit.js listeners=10 argc=10 n=2000000                         -0.16 %       ±2.76% ±3.67% ±4.78%
 events/ee-emit.js listeners=10 argc=2 n=2000000                           0.21 %       ±1.27% ±1.69% ±2.21%
 events/ee-emit.js listeners=10 argc=4 n=2000000                          -0.41 %       ±0.73% ±0.97% ±1.26%
 events/ee-emit.js listeners=1 argc=0 n=2000000                           -2.81 %       ±4.00% ±5.33% ±6.93%
 events/ee-emit.js listeners=1 argc=10 n=2000000                           2.13 %       ±4.61% ±6.15% ±8.02%
 events/ee-emit.js listeners=1 argc=2 n=2000000                           -1.90 %       ±4.31% ±5.74% ±7.47%
 events/ee-emit.js listeners=1 argc=4 n=2000000                           -1.07 %       ±3.87% ±5.15% ±6.72%
 events/ee-emit.js listeners=5 argc=0 n=2000000                           -0.70 %       ±1.96% ±2.62% ±3.42%
 events/ee-emit.js listeners=5 argc=10 n=2000000                          -0.25 %       ±1.93% ±2.57% ±3.35%
 events/ee-emit.js listeners=5 argc=2 n=2000000                            0.34 %       ±1.56% ±2.08% ±2.71%
 events/ee-emit.js listeners=5 argc=4 n=2000000                            0.20 %       ±1.81% ±2.41% ±3.14%
 events/ee-listener-count-on-prototype.js n=50000000                       2.43 %       ±4.79% ±6.45% ±8.53%
 events/ee-listeners.js raw='false' listeners=50 n=5000000                -0.02 %       ±2.45% ±3.29% ±4.32%
 events/ee-listeners.js raw='false' listeners=5 n=5000000                 -0.72 %       ±1.33% ±1.77% ±2.30%
 events/ee-listeners.js raw='true' listeners=50 n=5000000                 -0.42 %       ±1.57% ±2.09% ±2.72%
 events/ee-listeners.js raw='true' listeners=5 n=5000000                  -1.05 %       ±4.00% ±5.33% ±6.95%
 events/ee-once.js argc=0 n=20000000                                       0.22 %       ±1.26% ±1.68% ±2.18%
 events/ee-once.js argc=1 n=20000000                                      -1.10 %       ±2.20% ±2.96% ±3.90%
 events/ee-once.js argc=4 n=20000000                                      -1.49 %       ±2.47% ±3.30% ±4.32%
 events/ee-once.js argc=5 n=20000000                                      -1.39 %       ±2.39% ±3.20% ±4.19%
 events/eventtarget.js listeners=10 n=1000000                             -1.05 %       ±3.86% ±5.18% ±6.84%
 events/eventtarget.js listeners=1 n=1000000                               3.84 %       ±5.15% ±6.90% ±9.10%
 events/eventtarget.js listeners=5 n=1000000                              -2.07 %       ±2.20% ±2.93% ±3.81%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 25 comparisons, you can thus
expect the following amount of false-positive results:
  1.25 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.25 false positives, when considering a   1% risk acceptance (**, ***),
  0.03 false positives, when considering a 0.1% risk acceptance (***)

                                                                                           confidence improvement accuracy (*)   (**)  (***)
 streams/creation.js kind='duplex' n=50000000                                                              1.31 %       ±2.66% ±3.54% ±4.61%
 streams/creation.js kind='readable' n=50000000                                                     *      2.88 %       ±2.74% ±3.66% ±4.79%
 streams/creation.js kind='transform' n=50000000                                                          -2.21 %       ±3.85% ±5.13% ±6.69%
 streams/creation.js kind='writable' n=50000000                                                     *      2.06 %       ±1.83% ±2.44% ±3.17%
 streams/pipe.js n=5000000                                                                                 0.14 %       ±2.20% ±2.93% ±3.83%
 streams/pipe-object-mode.js n=5000000                                                                    -0.86 %       ±3.39% ±4.53% ±5.94%
 streams/readable-async-iterator.js sync='no' n=100000                                                     1.09 %       ±2.93% ±3.91% ±5.10%
 streams/readable-async-iterator.js sync='yes' n=100000                                                    3.48 %       ±4.84% ±6.46% ±8.45%
 streams/readable-bigread.js n=1000                                                                       -0.22 %       ±4.11% ±5.48% ±7.13%
 streams/readable-bigunevenread.js n=1000                                                                 -0.61 %       ±2.01% ±2.69% ±3.51%
 streams/readable-boundaryread.js type='buffer' n=2000                                                     1.18 %       ±3.43% ±4.57% ±5.98%
 streams/readable-boundaryread.js type='string' n=2000                                              *     -1.93 %       ±1.63% ±2.17% ±2.84%
 streams/readable-readall.js n=5000                                                                       -1.99 %       ±3.44% ±4.58% ±5.97%
 streams/readable-unevenread.js n=1000                                                              *     -2.89 %       ±2.88% ±3.86% ±5.08%
 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='no' n=2000000              *      1.51 %       ±1.31% ±1.74% ±2.27%
 streams/writable-manywrites.js len=1024 callback='no' writev='no' sync='yes' n=2000000                   -3.17 %       ±3.86% ±5.14% ±6.69%
 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='no' n=2000000                    0.90 %       ±2.22% ±2.95% ±3.85%
 streams/writable-manywrites.js len=1024 callback='no' writev='yes' sync='yes' n=2000000                  -0.66 %       ±3.36% ±4.48% ±5.84%
 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='no' n=2000000                    0.64 %       ±4.95% ±6.60% ±8.62%
 streams/writable-manywrites.js len=1024 callback='yes' writev='no' sync='yes' n=2000000                  -1.59 %       ±3.14% ±4.18% ±5.44%
 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='no' n=2000000                  -1.45 %       ±4.73% ±6.35% ±8.36%
 streams/writable-manywrites.js len=1024 callback='yes' writev='yes' sync='yes' n=2000000                 -1.61 %       ±2.79% ±3.72% ±4.84%
 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='no' n=2000000                    0.33 %       ±3.19% ±4.28% ±5.65%
 streams/writable-manywrites.js len=32768 callback='no' writev='no' sync='yes' n=2000000                   0.24 %       ±1.49% ±1.99% ±2.59%
 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='no' n=2000000                   1.07 %       ±2.15% ±2.86% ±3.74%
 streams/writable-manywrites.js len=32768 callback='no' writev='yes' sync='yes' n=2000000                  0.44 %       ±2.63% ±3.50% ±4.56%
 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='no' n=2000000                   1.48 %       ±1.49% ±1.99% ±2.59%
 streams/writable-manywrites.js len=32768 callback='yes' writev='no' sync='yes' n=2000000                 -0.28 %       ±1.12% ±1.49% ±1.94%
 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='no' n=2000000                  0.14 %       ±2.33% ±3.10% ±4.05%
 streams/writable-manywrites.js len=32768 callback='yes' writev='yes' sync='yes' n=2000000                 0.91 %       ±2.08% ±2.76% ±3.60%

Be aware that when doing many comparisons the risk of a false-positive
result increases. In this case there are 30 comparisons, you can thus
expect the following amount of false-positive results:
  1.50 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.30 false positives, when considering a   1% risk acceptance (**, ***),
  0.03 false positives, when considering a 0.1% risk acceptance (***)

github-actions · 2020-12-27T17:30:19Z

Commit Queue failed

- Loading data for nodejs/node/pull/36532
✔  Done loading data for nodejs/node/pull/36532
----------------------------------- PR info ------------------------------------
Title      lib: add primordials.SafeArrayIterator (#36532)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     aduh95:safe-array-iterator -> nodejs:master
Labels     author ready, lib / src
Commits    1
 - lib: add primordials.SafeArrayIterator
Committers 1
 - Antoine du Hamel 
PR-URL: https://github.com/nodejs/node/pull/36532
Reviewed-By: Rich Trott 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36532
Reviewed-By: Rich Trott 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - lib: add primordials.SafeArrayIterator
   ✔  Last GitHub Actions successful
   ℹ  Last Benchmark CI on 2020-12-26T10:16:57Z: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/804/console
   ℹ  Last Full PR CI on 2020-12-25T19:32:19Z: https://ci.nodejs.org/job/node-test-pull-request/35080/
- Querying data for job/node-test-pull-request/804/
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Tue, 15 Dec 2020 17:21:16 GMT
   ✔  Approvals: 1
   ✔  - Rich Trott (@Trott) (TSC): https://github.com/nodejs/node/pull/36532#pullrequestreview-557887739
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/447379775

PR-URL: nodejs#36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 · 2020-12-27T23:09:51Z

Landed in 0b6d307

Using an explicit constructor is necessary to avoid relying on `Array.prototype[Symbol.iterator]` and `%ArrayIteratorPrototype%.next`, which can be mutated by users. PR-URL: #36587 Refs: #36428 Refs: #36532 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: #36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

Using an explicit constructor is necessary to avoid relying on `Array.prototype[Symbol.iterator]` and `%ArrayIteratorPrototype%.next`, which can be mutated by users. PR-URL: #36587 Refs: #36428 Refs: #36532 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

PR-URL: nodejs#36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: #36532 Backport-PR-URL: #39446 Reviewed-By: Rich Trott <rtrott@gmail.com>

PR-URL: nodejs#36532 Backport-PR-URL: nodejs#39446 Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 added the blocked PRs that are blocked by other issues or PRs. label Dec 15, 2020

nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Dec 15, 2020

aduh95 commented Dec 15, 2020

View reviewed changes

lib/internal/buffer.js Outdated Show resolved Hide resolved

aduh95 mentioned this pull request Dec 18, 2020

repl: refactor to avoid unsafe array iteration #36444

Closed

2 tasks

mscdex previously requested changes Dec 20, 2020

View reviewed changes

mscdex mentioned this pull request Dec 20, 2020

lib: make safe primordials safe to construct #36428

Closed

2 tasks

ExE-Boss reviewed Dec 20, 2020

View reviewed changes

lib/internal/buffer.js Outdated Show resolved Hide resolved

lib/internal/buffer.js Outdated Show resolved Hide resolved

aduh95 mentioned this pull request Dec 20, 2020

buffer: make FastBuffer safe to construct #36587

Closed

2 tasks

This comment has been minimized.

Sign in to view

aduh95 requested a review from mscdex December 21, 2020 13:08

aduh95 force-pushed the safe-array-iterator branch from 9a728a4 to def229a Compare December 22, 2020 19:21

Trott reviewed Dec 23, 2020

View reviewed changes

Trott approved these changes Dec 23, 2020

View reviewed changes

aduh95 force-pushed the safe-array-iterator branch from def229a to b975a4a Compare December 25, 2020 19:03

aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed blocked PRs that are blocked by other issues or PRs. labels Dec 25, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 25, 2020

aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 27, 2020

github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 27, 2020

github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 27, 2020

lib: add primordials.SafeArrayIterator

0b6d307

PR-URL: nodejs#36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 force-pushed the safe-array-iterator branch from b975a4a to 0b6d307 Compare December 27, 2020 23:09

aduh95 merged commit 0b6d307 into nodejs:master Dec 27, 2020

aduh95 deleted the safe-array-iterator branch December 27, 2020 23:09

danielleadams pushed a commit that referenced this pull request Jan 12, 2021

lib: add primordials.SafeArrayIterator

181bad5

PR-URL: #36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

danielleadams mentioned this pull request Jan 12, 2021

v15.6.0 proposal #36889

Merged

targos added the backport-blocked-v14.x label May 25, 2021

aduh95 added a commit to aduh95/node that referenced this pull request Jul 20, 2021

lib: add primordials.SafeArrayIterator

c53b4ad

PR-URL: nodejs#36532 Reviewed-By: Rich Trott <rtrott@gmail.com>

aduh95 mentioned this pull request Jul 20, 2021

[v14.x backport] debugger patches #39446

Closed

aduh95 added backport-open-v14.x and removed backport-blocked-v14.x labels Jul 20, 2021

richardlau pushed a commit that referenced this pull request Jul 22, 2021

lib: add primordials.SafeArrayIterator

89b353d

PR-URL: #36532 Backport-PR-URL: #39446 Reviewed-By: Rich Trott <rtrott@gmail.com>

richardlau pushed a commit that referenced this pull request Jul 22, 2021

lib: add primordials.SafeArrayIterator

d8cc2ff

PR-URL: #36532 Backport-PR-URL: #39446 Reviewed-By: Rich Trott <rtrott@gmail.com>

richardlau mentioned this pull request Jul 22, 2021

v14.17.4 proposal #39466

Merged

targos added backported-to-v14.x and removed backport-open-v14.x commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Sep 5, 2021

foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021

lib: add primordials.SafeArrayIterator

9d57349

PR-URL: nodejs#36532 Backport-PR-URL: nodejs#39446 Reviewed-By: Rich Trott <rtrott@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib: add primordials.SafeArrayIterator #36532

lib: add primordials.SafeArrayIterator #36532

aduh95 commented Dec 15, 2020 •

edited

mscdex left a comment •

edited

This comment has been minimized.

This comment has been minimized.

aduh95 commented Dec 21, 2020 •

edited

aduh95 commented Dec 22, 2020 •

edited

Trott left a comment

Trott commented Dec 23, 2020

Trott left a comment

nodejs-github-bot commented Dec 25, 2020

aduh95 commented Dec 26, 2020

github-actions bot commented Dec 27, 2020

aduh95 commented Dec 27, 2020

lib: add primordials.SafeArrayIterator #36532

lib: add primordials.SafeArrayIterator #36532

Conversation

aduh95 commented Dec 15, 2020 • edited

Checklist

mscdex left a comment • edited

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

aduh95 commented Dec 21, 2020 • edited

aduh95 commented Dec 22, 2020 • edited

Trott left a comment

Choose a reason for hiding this comment

Trott commented Dec 23, 2020

Trott left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Dec 25, 2020

aduh95 commented Dec 26, 2020

github-actions bot commented Dec 27, 2020

aduh95 commented Dec 27, 2020

aduh95 commented Dec 15, 2020 •

edited

mscdex left a comment •

edited

aduh95 commented Dec 21, 2020 •

edited

aduh95 commented Dec 22, 2020 •

edited