Review requested:

• @nodejs/streams

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

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

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

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

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

@nodejs/build this (and other PRs) are failing for a while for an unrelated test on windows.

@nodejs/build this (and other PRs) are failing for a while for an unrelated test on windows.

This seems to be affecting only streams PRs, no? How confident are you that the failure isn't related?

@Trott it's a test on windows that doesn't use streams or promises. Maybe @targos who wrote the test knows?

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

Surprisingly (and IMO problematically), it appears that Jenkins does not rebase the pull request against the branch to which it is landing. You might try rebasing, force-pushing, and running CI again, in case the test was fixed by a commit that's landed in the last 48 hours or something like that.

Surprisingly (and IMO problematically), it appears that Jenkins does not rebase the pull request against the branch to which it is landing. You might try rebasing, force-pushing, and running CI again, in case the test was fixed by a commit that's landed in the last 48 hours or something like that.

Another possibility might be putting origin/master in the rebase-onto field in Jenkins and not relying on the default.

Surprisingly (and IMO problematically), it appears that Jenkins does not rebase the pull request against the branch to which it is landing. You might try rebasing, force-pushing, and running CI again, in case the test was fixed by a commit that's landed in the last 48 hours or something like that.

:O that's... very surprising thanks.

So the option in Jenkins is ignored?

Surprisingly (and IMO problematically), it appears that Jenkins does not rebase the pull request against the branch to which it is landing. You might try rebasing, force-pushing, and running CI again, in case the test was fixed by a commit that's landed in the last 48 hours or something like that.

Eh what? There's definitely code in the CI jobs to do the rebasing. It won't re-rebase if you resume a job though.

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

Edit: please do not abort me! I wanna see if that's the issue

This one is without a rebase just another request-ci

Now another test is failing:

AssertionError [ERR_ASSERTION]: ifError got unwanted exception: Command failed: "C:\workspace\node-test-binary-windows-js-suites\node\Release\node.exe" "C:\workspace\node-test-binary-windows-js-suites\node\test\fixtures\print-chars-from-buffer.js" 1048576 > "C:\workspace\node-test-binary-windows-js-suites\node\test.tmp.571\stdout.txt"
Access is denied.

https://ci.nodejs.org/job/node-test-binary-windows-js-suites/13335/RUN_SUBSET=2,nodes=win10-COMPILED_BY-vs2019/testReport/junit/(root)/test/parallel_test_stdout_to_file/

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

Output of stdout from failure:

<--- Last few GCs --->

[8412:0000015ECFDE8D30]       20 ms: Mark-sweep 2.6 (4.9) -> 2.5 (4.9) MB, 2.5 / 0.0 ms  (average mu = 0.652, current mu = 0.652) allocation failure GC in old space requested
[8412:0000015ECFDE8D30]       26 ms: Mark-sweep 2.8 (5.1) -> 2.8 (5.4) MB, 3.7 / 0.0 ms  (average mu = 0.525, current mu = 0.374) allocation failure GC in old space requested


<--- JS stacktrace --->


#
# Fatal javascript OOM in Reached heap limit
#


node:assert:123
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Wrong exit code of 2147483651! Expected 134 for abort
    at C:\workspace\node-test-binary-windows-js-suites\node\test\parallel\test-windows-failed-heap-allocation.js:29:10
    at C:\workspace\node-test-binary-windows-js-suites\node\test\common\index.js:401:15
    at ChildProcess.exithandler (node:child_process:395:5)
    at ChildProcess.emit (node:events:522:28)
    at maybeClose (node:internal/child_process:1090:16)
    at Process.ChildProcess._handle.onexit (node:internal/child_process:302:5) {
  generatedMessage: false,
  code: 'ERR_ASSERTION',
  actual: 2147483651,
  expected: 134,
  operator: 'strictEqual'
}

Node.js v18.0.0-pre

It looks like v8 does return this exit code on OOM and Node doesn't get to intercept it - investigating.

Like NearHeapLimitCallback isn't called and V8 exits immediately because the memory grows too quickly but only on windows.

This is pretty easy to reproduce by increasing the memory pressure, if I increase the heap size I get 134 consistently but without it I get SIGTRAP pretty consistently.

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

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

Eh what? There's definitely code in the CI jobs to do the rebasing. It won't re-rebase if you resume a job though.

Oh, maybe that was the source of my mistake.

This is pretty easy to reproduce by increasing the memory pressure, if I increase the heap size I get 134 consistently but without it I get SIGTRAP pretty consistently.

Does it make sense to move the test from parallel to sequential in that case to make sure it's not competing with other processes for resources?

This is pretty easy to reproduce by increasing the memory pressure, if I increase the heap size I get 134 consistently but without it I get SIGTRAP pretty consistently.

Does it make sense to move the test from parallel to sequential in that case to make sure it's not competing with other processes for resources?

Although I guess it raises the question: SHOULD it be sensitive to memory pressure?

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

Does it make sense to move the test from parallel to sequential in that case to make sure it's not competing with other processes for resources?

I think just not synchronously causing an OOM in V8 so that it calls the NearHeapLimit thing should be enough?

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

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

Landed in c15639d...7752eed 🎉