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

Calling async function recursively stops (randomly?) after 126 calls #1055

Closed
SharonBrizinov opened this issue Jul 15, 2023 · 9 comments
Closed
Labels
bug Something isn't working

Comments

@SharonBrizinov
Copy link
Contributor

Bug Description

I was playing with async function in Hermes and found something strange - when calling an async function recursively it will execute 126 times while in fact it should run forever, until exhausting the call stack.

Version

Hermes version: v0.12.0 and latest main branch commit 36195b3
OS version (if any): Ubutnu v22.04

Steps To Reproduce

Run the following code snippet

async function foo(i) {
 print(i);
 foo(i+1);
}
foo(0);

Current output

0
1
2
...
...
...
125
126

The Expected Behavior

run until exhausting the call stack at approx ~80654 recursive calls

0
1
2
...
...
...
80653
80654
Uncaught RangeError: Maximum call stack size exceeded
    at foo (/tmp/hermes-input.js:3:5)
    at foo (/tmp/hermes-input.js:3:5)
    at foo (/tmp/hermes-input.js:3:5)
@SharonBrizinov SharonBrizinov added the bug Something isn't working label Jul 15, 2023
@fbmal7
Copy link
Contributor

fbmal7 commented Jul 17, 2023

This is not a random number, it seems you are running into the native stack limit error. You can read more about the details here #135 (comment)

However, at a first glance of this code, I'm not sure why this is recursing on the native stack.

@SharonBrizinov
Copy link
Contributor Author

@fbmal7 thanks for your quick answer. But why is it running on the native stack? I'm not using any native functions here. Also, why am I not getting a RangerError for this case?

@fbmal7
Copy link
Contributor

fbmal7 commented Jul 17, 2023

Not sure yet what's going on, but this does look like a bug.

async function foo(i) {
  print(i);
  foo(i+1);
}
foo(0);
print('after');

The print statement for 'after' is executed after 126 is printed out, which definitely is wrong. Thanks for raising this.

@tmikov
Copy link
Contributor

tmikov commented Jul 17, 2023

@fbmal7 if I run the example with JSC, I get:

0
1
2
...
...
10641
10642
after

But again no error.

@avp
Copy link
Contributor

avp commented Jul 17, 2023

@SharonBrizinov what command did you run to get your "Expected Output"?

If I run the code with d8 test.js, I get:

/Users/avp/test.js:1: RangeError: Maximum call stack size exceeded
async function foo(i) {
                  ^
RangeError: Maximum call stack size exceeded
    at foo (/Users/avp/test.js:1:19)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)
    at foo (/Users/avp/test.js:3:9)

1 pending unhandled Promise rejection(s) detected.

Indicating that the error causing the nonzero exit code is for unhandled Promise rejection, not actually for stack overflow. Hermes does have a mechanism for setting up unhandled Promise rejection tracking, but it's not enabled by default on the CLI tool (much like JSC, based on tmikov's comment above).

https://github.com/facebook/hermes/blob/main/lib/VM/JSLib/HermesInternal.cpp#L697 explains how to set up Promise rejection tracking: one can use the setPromiseRejectionTrackingHook and enablePromiseRejectionTracker to catch the unhandled Promise rejections.

@SharonBrizinov
Copy link
Contributor Author

Thanks @avp! so the only question remains is why it's using the native stack for this case?

@avp
Copy link
Contributor

avp commented Jul 18, 2023

Internally, async functions compile to uses of generators, and the generator next function is an internal Hermes native function.

@SharonBrizinov
Copy link
Contributor Author

Interesting, thanks @avp

@tmikov
Copy link
Contributor

tmikov commented Jul 18, 2023

FWIW, the limit will increase when we deploy native stack checking. The current limit is deliberately very conservative, to avoid the possibility of native stack overflow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants