Skip to content
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.

Sign up for GitHub

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

Jump to bottom

test-buffer-failed-alloc-typed-arrays test failures #53085

Closed
AdamMajer opened this issue May 21, 2024 · 4 comments · Fixed by #53099
Closed

test-buffer-failed-alloc-typed-arrays test failures #53085

AdamMajer opened this issue May 21, 2024 · 4 comments · Fixed by #53099
Labels
test Issues and PRs related to the tests.

Comments

@AdamMajer
Copy link
Contributor

Version

22.1.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

No response

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

No response

What do you see instead?

crash on multiple architectures on openSUSE Tumbleweed

[ 327s] not ok 1180 parallel/test-buffer-failed-alloc-typed-arrays
[ 327s] ---
[ 327s] duration_ms: 45388.77800
[ 327s] severity: crashed
[ 327s] exitcode: -9
[ 327s] stack: |-
[ 327s] ...

Additional information

No response
@AdamMajer
Copy link
Contributor Author

It seems to be running out of memory ...

> /usr/bin/time ./node22 test/parallel/test-buffer-failed-alloc-typed-arrays.js 
(node:20905) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `node22 --trace-deprecation ...` to show where the warning was created)
3.30user 9.69system 0:12.93elapsed 100%CPU (0avgtext+0avgdata 19586856maxresident)k
0inputs+0outputs (0major+6511685minor)pagefaults 0swaps

@VoltrexKeyva VoltrexKeyva added the test Issues and PRs related to the tests. label May 22, 2024
@AdamMajer
Copy link
Contributor Author

This is a regression.

With nodejs 21.7.2, the test uses only normally expected amount of memory,

/usr/bin/time ./node21 test/parallel/test-buffer-failed-alloc-typed-arrays.js
(node:15146) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use node21 --trace-deprecation ... to show where the warning was created)
0.05user 0.01system 0:00.07elapsed 100%CPU (0avgtext+0avgdata 53176maxresident)k
0inputs+0outputs (0major+3173minor)pagefaults 0swaps

basically, 20G vs. 50M

@targos
Copy link
Member

targos commented May 22, 2024

node/test/parallel/test-buffer-failed-alloc-typed-arrays.js

Line 15 in 4a54a80

const sizes = [1e10, 0, 0.1, -1, 'a', undefined, null, NaN];

The test tries to allocate a buffer of size 1e10. This used to fail but is now possible after a V8 update.

@AdamMajer
Copy link
Contributor Author

Looks like changing this to 1e20 will fix this issue and keep the test around.

AdamMajer added a commit to AdamMajer/node that referenced this issue May 22, 2024
@AdamMajer
test: increase allocation so it fails for the test
53450cd 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: nodejs#53085
@AdamMajer AdamMajer mentioned this issue May 22, 2024
Merged
nodejs-github-bot pushed a commit that referenced this issue May 24, 2024
@AdamMajer
test: increase allocation so it fails for the test
501546e 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: #53085
PR-URL: #53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
targos pushed a commit that referenced this issue Jun 1, 2024
@AdamMajer @targos
test: increase allocation so it fails for the test
a371dea 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: #53085
PR-URL: #53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
eliphazb pushed a commit to eliphazb/node that referenced this issue Jun 20, 2024
@AdamMajer @eliphazb
test: increase allocation so it fails for the test
a1a5d71 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: nodejs#53085
PR-URL: nodejs#53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this issue Jun 22, 2024
@AdamMajer @bmeck
test: increase allocation so it fails for the test
76a4b0b 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: nodejs#53085
PR-URL: nodejs#53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
@AdamMajer @marco-ippolito
test: increase allocation so it fails for the test
c74f130 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: #53085
PR-URL: #53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
marco-ippolito pushed a commit that referenced this issue Jul 19, 2024
@AdamMajer @marco-ippolito
test: increase allocation so it fails for the test
0051d1c 
It used to be impossible to allocate 1e10 bytes but with a v8 update
this allocation can succeed. This results in 2x10GB allocations and
failure to test the failure :-)

Increase allocation to 1e20 bytes, which should fail for some time,
returning the test to using only 50MB at runtime.

Fixes: #53085
PR-URL: #53099
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test: increase allocation so it fails for the test AdamMajer/node
3 participants
@AdamMajer @targos @VoltrexKeyva