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.

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

worker_threads emergency aborted (std::invalid_argument exception from std::stoull) #46223

Closed
a-shvedov opened this issue Jan 15, 2023 · 4 comments · Fixed by #46290
Closed
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. confirmed-bug Issues with confirmed bugs.

Comments

@a-shvedov
Copy link

Version

node-v19.3.0

Platform

5.15.78-2.el7.3.x86_64 (RED OS release MUROM 7.3.2 - Centos based system)

Subsystem

Worker threads

What steps will reproduce the bug?

thread_minimize.js:

const { Worker } = require('worker_threads');
const path = require('path');
new Worker(path.join(__dirname), {
// new Worker(path.join(__dirname, 'test.js'), {
execArgv: [ '--cpu-prof-interval', process.TEST ] });

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

Standart build compiled with: gcc (version 8.3.1 2019112)
Configure params: --prefix="/target/dir"
Builded binary info: node: ELF 64-bit LSB executable, x86-64, version 1 (GNU/Linux), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, BuildID[sha1]=1bf428404ef62dd8b5c3f952c02e1d1e8da610a9, for GNU/Linux 3.2.0

What is the expected behavior?

No response

What do you see instead?

Reproduce an error with the following options: ASAN_OPTIONS=verbosity=1:detect_leaks=1:symbolize=1 LD_PRELOAD=/usr/lib64/libasan.so.5.0.0 /home/user/target/node-v19.0.1/node-v19.0.1/builded/bin/node ./thread_minimize.js

Asan log:

MemToShadow(shadow): 0x00008fff7000 0x000091ff6dff 0x004091ff6e00 0x02008fff6fff
redzone=16
max_redzone=2048
quarantine_size_mb=256M
thread_local_quarantine_size_kb=1024K
malloc_context_size=30
SHADOW_SCALE: 3
SHADOW_GRANULARITY: 8
SHADOW_OFFSET: 0x7fff8000
==1664269==Installed the sigaction for signal 11
==1664269==Installed the sigaction for signal 7
==1664269==Installed the sigaction for signal 8
==1664269==T0: stack [0x7fff1476b000,0x7fff14f6b000) size 0x800000; local=0x7fff14f68584
==1664269==AddressSanitizer Init done
==1664269==T1: stack [0x7faa62000000,0x7faa627fef00) size 0x7fef00; local=0x7faa627fee04
==1664269==T2: stack [0x7faa617ff000,0x7faa61ffdf00) size 0x7fef00; local=0x7faa61ffde04
==1664269==T3: stack [0x7faa60ffe000,0x7faa617fcf00) size 0x7fef00; local=0x7faa617fce04
==1664269==T4: stack [0x7faa607fd000,0x7faa60ffbf00) size 0x7fef00; local=0x7faa60ffbe04
==1664269==T5: stack [0x7faa5fffc000,0x7faa607faf00) size 0x7fef00; local=0x7faa607fae04
==1664269==T6: stack [0x7faa63026000,0x7faa6302cf00) size 0x6f00; local=0x7faa6302ce04
terminate called after throwing an instance of 'std::invalid_argument'
  what():  stoull
Aborted

Additional information

No response

@bnoordhuis
Copy link
Member

Looks to have been introduced in 436f4de from 2018. The easy fix is to replace stoull(value) with strtoull(value.c_str(), nullptr, 10). Want to send a pull request?

Open question: how to handle invalid inputs? The kInteger case uses atoll() and simply ignores errors. atoll() is basically strtoll() with base=10.

@bnoordhuis
Copy link
Member

bnoordhuis commented Jan 18, 2023

Forgot to mention, I can easily reproduce like this:

$ node --cpu-prof-interval boom -p 42
libc++abi.dylib: terminating with uncaught exception of type std::invalid_argument: stoull: no conversion
Abort trap: 6

So yes, confirmed bug.

@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. c++ Issues and PRs that require attention from people who are familiar with C++. labels Jan 18, 2023
@bnoordhuis
Copy link
Member

#46290

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Jan 20, 2023
Replace stoull() with strtoull(). The former throws an exception when
the input is malformed, the latter doesn't.

Fixes: nodejs#46223
@a-shvedov
Copy link
Author

#46290

Thanks for the quick solution to this problem!

nodejs-github-bot pushed a commit that referenced this issue Jan 22, 2023
Replace stoull() with strtoull(). The former throws an exception when
the input is malformed, the latter doesn't.

Fixes: #46223
PR-URL: #46290
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
ruyadorno pushed a commit that referenced this issue Feb 1, 2023
Replace stoull() with strtoull(). The former throws an exception when
the input is malformed, the latter doesn't.

Fixes: #46223
PR-URL: #46290
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this issue Mar 3, 2023
Replace stoull() with strtoull(). The former throws an exception when
the input is malformed, the latter doesn't.

Fixes: #46223
PR-URL: #46290
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this issue Mar 5, 2023
Replace stoull() with strtoull(). The former throws an exception when
the input is malformed, the latter doesn't.

Fixes: #46223
PR-URL: #46290
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. confirmed-bug Issues with confirmed bugs.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants