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

Call stack may need increasing #956

Closed
jacobp100 opened this issue Apr 3, 2023 · 4 comments
Closed

Call stack may need increasing #956

jacobp100 opened this issue Apr 3, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@jacobp100
Copy link
Contributor

Bug Description

I switched my app to Hermes and started seeing some errors in Sentry. My app uses MathJax to do rendering - which is where the errors are originated from

I've pinned it down to the call stack just being too small

I've attached a repo that demonstrates the problem (run yarn, yarn build, then run out.js node or Hermes)

Node runs perfectly fine. Hermes has a stack overflow. I tested increasing the stack size, and that did fix the problem

I'm not really sure what the exact limit should be - the user from this error appeared to be having nesting square roots, presumably for the pretty pattern they produce. They got 11 levels deep before the error occurred with Hermes. I can get to about 70 with JSC

I suppose there will be a few other applications where the stack size depends on user input

code example:

https://github.com/jacobp100/mathjax-hermes

The Expected Behavior

@jacobp100 jacobp100 added the bug Something isn't working label Apr 3, 2023
@tmikov
Copy link
Contributor

tmikov commented Apr 3, 2023

Hermes compiled for desktop has a huge stack compared to all other engines, which you can verify by running this:

let level = 0;
function foo() {
    ++level;
    foo();
}
try {
    foo();
} catch (e) {
    print("Level", level);
    print(e);
}

So, in order to pin this down, I have a few questions:

  • Which version of Hermes?
  • Can you attach a copy of the problematic stack that you captured? Which part of the code throws it?
  • How did you increase it and to what value?
  • Why do you believe that this is a bug? Did Hermes crash?

@jacobp100
Copy link
Contributor Author

jacobp100 commented Apr 3, 2023

I built it off the main branch. I increased the limit of MAX_NATIVE_CALL_FRAME_DEPTH from 128 (I think) to 1024

The example branch shows (most of) the real-world scenario - so the stack traces are accurate there. But I'll add them below

Stack trace
Uncaught RangeError: Maximum call stack size exceeded (native stack depth)
    at anonymous (out.js:3341:23)
    at anonymous (out.js:3360:32)
    at anonymous (out.js:3409:55)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:12695:42)
    at anonymous (out.js:12655:37)
    at map (native)
    at CommonWrapper2 (out.js:12654:50)
    at apply (native)
    at SVGWrapper2 (out.js:13176:54)
    at apply (native)
    at class_1 (out.js:13698:37)
    at apply (native)
    at SVGmrow2 (out.js:13847:49)
    at apply (native)
    at class_2 (out.js:13803:51)
    at apply (native)
    at SVGinferredMrow2 (out.js:13860:49)
    at anonymous (out.js:3409:79)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:12695:42)
    at anonymous (out.js:12655:37)
    at map (native)
    at CommonWrapper2 (out.js:12654:50)
    at apply (native)
    at SVGWrapper2 (out.js:13176:54)
    at apply (native)
    at class_1 (out.js:15507:37)
    at apply (native)
    at SVGmsqrt2 (out.js:15608:54)
    at anonymous (out.js:3409:79)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:12695:42)
    at anonymous (out.js:12655:37)
    at map (native)
    at CommonWrapper2 (out.js:12654:50)
    at apply (native)
    at SVGWrapper2 (out.js:13176:54)
    at apply (native)
    at class_1 (out.js:13698:37)
    at apply (native)
    ... skipping 196 frames
    at SVGmrow2 (out.js:13847:49)
    at anonymous (out.js:3409:79)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:12695:42)
    at anonymous (out.js:12655:37)
    at map (native)
    at CommonWrapper2 (out.js:12654:50)
    at apply (native)
    at SVGWrapper2 (out.js:13176:54)
    at apply (native)
    at class_1 (out.js:13698:37)
    at apply (native)
    at SVGmrow2 (out.js:13847:49)
    at apply (native)
    at class_2 (out.js:13803:51)
    at apply (native)
    at SVGinferredMrow2 (out.js:13860:49)
    at anonymous (out.js:3409:79)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:12695:42)
    at anonymous (out.js:12655:37)
    at map (native)
    at CommonWrapper2 (out.js:12654:50)
    at apply (native)
    at SVGWrapper2 (out.js:13176:54)
    at apply (native)
    at class_1 (out.js:13474:51)
    at apply (native)
    at SVGmath2 (out.js:13549:49)
    at anonymous (out.js:3409:79)
    at apply (native)
    at anonymous (out.js:3398:72)
    at apply (native)
    at anonymous (out.js:12088:35)
    at anonymous (out.js:30161:42)
    at anonymous (out.js:11679:27)
    at anonymous (out.js:11655:21)
    at anonymous (out.js:3249:97)
    at anonymous (out.js:9699:39)
    at anonymous (out.js:9765:41)
    at anonymous (out.js:3239:47)
    at anonymous (out.js:9931:24)
    at anonymous (out.js:30314:26)
    at global (out.js:30322:3)

It's normal MathJax code - it's the input that causes the stack to overflow

Looking at this a bit more, I think it's the interaction with natively implemented functions causing this. This code demonstrates the problem:-

var level = 0;
function foo() {
  ++level;
  [0].map(foo);
}
try {
  foo();
} catch (e) {
  print("Level", level);
}

Gives 128. I get over 15k in JSC.

@fbmal7
Copy link
Contributor

fbmal7 commented Apr 3, 2023

@jacobp100 What you're running into has been observed before. You can read Tzvetan's comment here #135 (comment) but the native stack limit is set to 128.

@jacobp100
Copy link
Contributor Author

Yep - that looks like the same issue. I'll close this. Thanks

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

3 participants