facebook/hermes

RangeError: Maximum call stack size exceeded when require/load very big json file

dongyuwei opened this issue ยท 83 comments

https://github.com/dongyuwei/tiny_english_dictionary/blob/hermes/trie-service.js#L2
const words = require('./words_with_frequency_and_translation_and_ipa.json'); will throw error:

RangeError: Maximum call stack size exceeded, js engine:hermes

The json file size is 21M, while its data structure is very simple.

Steps to reproduce the bug:

  1. yarn start
  2. start an android emulator
  3. yarn android

Tested with yarn 1.17.3 and nodejs 12.12.0 on Mac 10.14.5.

Exception call stack:
image

Thanks for reporting this. It seems like we have a problem with compiling large JSON files. We are looking into it and will post an update here.

Meanwhile, as a workaround can you parse as a string with JSON.parse()?

Thanks for your reply, I'll try and let you know today. On road now.

I just verified JSON.parse works. See https://github.com/dongyuwei/tiny_english_dictionary/blob/hermes/trie-service.js#L18, I use react-native-local-resource to load the stringified json txt, then use JSON.parse to decode it.

In my case I have a 14MB file which builds fine in debug mode but Hermes hangs when trying to optimize. I had to turn off hermes optimizations for the build to complete. Let me know if I can help to somehow debug this for you.

There is a unit test to parse json: /unittests/Parser/JSONParserTest.cpp, but I don't know how to run the test.

@tmikov can you modify and run the test with my big json file words_with_frequency_and_translation_and_ipa.json? From the call stack of the exception, can't see any direct relationship with hermes.

We investigated this when it was reported and we believe we understand the bug. We haven't forgotten it, and we have been workin to address it, but progress has been slow because the fix is non-trivial. There are two separate problems:

  • Without optimizations, the generated code for large nested object literals consume inordinate amount of register stack, causing the stack overflow exception.
  • With optimizations, the stack usage is much better, but one particular optimization pass exhibits degenerate behavior on the same kind of object literals.

The workaround is to use JSON.parse() with a string or a resource.

I will look into escalating this, so the fix can land sooner.

This happened to me when I upgraded my react-native https://react-native-community.github.io/upgrade-helper/?from=0.63.4&to=0.64.0

 WARN  Require cycle: node_modules/core-js/internals/microtask.js -> node_modules/core-js/internals/microtask.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Fetch.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 ERROR  RangeError: Maximum call stack size exceeded, js engine: hermes
 LOG  Running "ny_app" with {"rootTag":1}
 ERROR  Invariant Violation: "ny_app" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called., js engine: hermes

Happens both on android and ios.

This happened to me when I upgraded my react-native https://react-native-community.github.io/upgrade-helper/?from=0.63.4&to=0.64.0

 WARN  Require cycle: node_modules/core-js/internals/microtask.js -> node_modules/core-js/internals/microtask.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Fetch.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 ERROR  RangeError: Maximum call stack size exceeded, js engine: hermes
 LOG  Running "ny_app" with {"rootTag":1}
 ERROR  Invariant Violation: "ny_app" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called., js engine: hermes

Happens both on android and ios.

Setting inlineRequires: false in metro.config.js fixed it for me

We investigated this when it was reported and we believe we understand the bug. We haven't forgotten it, and we have been workin to address it, but progress has been slow because the fix is non-trivial. There are two separate problems:

  • Without optimizations, the generated code for large nested object literals consume inordinate amount of register stack, causing the stack overflow exception.
  • With optimizations, the stack usage is much better, but one particular optimization pass exhibits degenerate behavior on the same kind of object literals.

The workaround is to use JSON.parse() with a string or a resource.

I will look into escalating this, so the fix can land sooner.

Is there any update on the fix?

I have a 13mb JSON file which should be loaded locally. I tried basically everything, but it always gives me this error... Can someone help me? Any working work-around for this? Or a fix somehow? Thank you... @tmikov

@traingethermarc the workaround is to use JSON.parse().

This happened to me when I upgraded my react-native https://react-native-community.github.io/upgrade-helper/?from=0.63.4&to=0.64.0

 WARN  Require cycle: node_modules/core-js/internals/microtask.js -> node_modules/core-js/internals/microtask.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Fetch.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 ERROR  RangeError: Maximum call stack size exceeded, js engine: hermes
 LOG  Running "ny_app" with {"rootTag":1}
 ERROR  Invariant Violation: "ny_app" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called., js engine: hermes

Happens both on android and ios.

This is happening to one of the apps I'm working on (1) BUT it does not import any big JSON files. On the other hand, the other app I'm working on (2) doesn't experience this, could it be related to auth0? because my former uses auth0 but the latter does not.

it has been two years. And this issue still occurs for the latest 0.9 version hermes.

What's hermes' max call stack size? I'm getting max call stack size errors for recursive functions as deep as just 60 calls.

@cristianoccazinsp For JS to JS recursion, the maximum depth is dynamic and depends on the size of every frame and the size of the register stack, which is configurable. In the default configuration, the following code performs 55,000 recursive invocations without optimization, and 87,000 with optimization:

let level = 0;
function foo() {
    ++level;
    foo();
}

try {
    foo();
} catch (e) {
    print("Level", level);
    print(e);
}

Hermes has a separate fixed stack limit for native calls, which is set to 128. So, if you change the recursive invocation from foo() to foo.call(), it throws after 128 recursive calls.

Native stack errors is just the crash I'm facing right now. Sigh, 128 is so small! This used to work fine with JSC and now blows up with Hermes. Sadly, the code is for an HTML parsing library that relies on recursion so 128 will definitely cause issues as HTMLs could be huge.

Keep in mind that the low number is only for cases involving recursion through a native function. The number is deliberately very conservative to guarantee that a native stack overflow will never occur. It is even lower in some build modes, because the native stack (which is not controlled by Hermes) can and does overflow quite easily.

The limit can be increased by changing this line:

There is some work in progress for obtaining and checking the actual stack size, which on most cases will result in significantly increased depth, but no promises when it will land.

I'm not entirely sure why I get native stack depth errors even though the recursion happens entirely in JS. Does calls to .map and "native" methods count towards the native stack?

Yes. Most "standard library" functions are implemented in C++ and unfortunately count as native.

The best way to address this is to check whether the native stack is actually overflowing, instead of setting artificial hard limits. I will see if I can finish that work.

@tmikov I replaced all .map calls with simple for a in b loops and the recursion errors went away (even though I'm still using recursive calls). Funny thing is, the moment the screen gets unmounted, another native stack depth error occurs from a totally different place (react-navigation stack and animations).

With this comment I want to raise awareness of this issue and show that there is a high demand for it to be fixed. (Even though there is a workaround to import as string and then use JSON.parse)

Can you detail how to use the workaround @aikewoody? I'm still getting this crash when using require("pathToJson") before I get the change to parse it

@kdthomas2121
To be honest I don't fully remember what the solution was exactly.

One trick that I used is giving the json file a .txt extension so that it wouldn't be automatically parsed.

This is the code I ended up with (React Native + Expo):
JSON.parse(await FileSystem.readAsStringAsync('~/assets/data/list-production.txt'));

Same issue occurs for big JS/TS file.

Is there any update about this issue?

@kdthomas2121 To be honest I don't fully remember what the solution was exactly.

One trick that I used is giving the json file a .txt extension so that it wouldn't be automatically parsed.

This is the code I ended up with (React Native + Expo): JSON.parse(await FileSystem.readAsStringAsync('~/assets/data/list-production.txt'));

Every chance you don't remember again but does this require putting the json file into the device local storage?

Also adding a comment to ask for a fix to this.

@tmikov how can we tell if HERMES_LIMIT_STACK_DEPTH is defined in the build we are using? https://github.com/facebook/hermes/blob/main/include/hermes/VM/Runtime.h#L918-L921

I ask because we're running into this same problem for a very large react-native app when running locally. I'm curious how we can be sure that we are using the predefined max for this or not.

@mikeduminy sorry, I have lost the context here. What kind of error are you getting and when?

@tmikov

Context
We have a large react-native app that uses code splitting to lazily load code as needed. Locally we use Metro which doesn't support code splitting -- it transforms all dynamic imports into regular requires. This causes the whole app to be a single, large bundle. We also use the Hermes runtime in development (this ensures we catch any engine-specific differences).

Problem
When this large bundle (100+ mb) is loaded the runtime throws an error:
RangeError: Maximum call stack size exceeded, js engine: hermes

Investigation
The only way we're able to make it work again (with metro + Hermes runtime) is by commenting out a bunch of these dynamic requires which are causing the bundle to grow so much. Importantly, by commenting them out at random, we are able to get it running again. This led us to believe that it's probably not about specific parts of our code but rather the sheer quantity of code.

The reason I asked about HERMES_LIMIT_STACK_DEPTH is because in it can cause MAX_NATIVE_CALL_FRAME_DEPTH to be very low. I'd like to not have to adjust internal limits within hermes to fix the problem but we're getting desperate.

@mikeduminy got it. Here are my first thoughts:

  • It is very unlikely that HERMES_LIMIT_STACK_DEPTH is set - it is only enabled in sanitizer builds of Hermes, which you are almost certainly not running.
  • This actually looks like a JS stack overflow, not a native stack overflow.

Are you getting a stack trace when it happens and what does it look like?

@tmikov yeah, I think you're right about the being a JS stack overflow. I can move the discussion to a different thread if needed?

This is the error I get:

Maximum call stack size exceeded

no stack

To be clear, this doesn't happen when using JSC as a runtime. Is there a way to get a stack trace? Perhaps by building hermes locally or enabling some debug flags?

This error happens for example with the ajv module to verify JSON schemas. I checked their code and see that it would be very difficult to change the code logic to prevent many stacked function calls.

To see the maximum stack trace, this code might help to find it:

const fn = (i) => {
  console.log("i =", i);
  fn(i + 1);
};

fn(0);

@mikeduminy something strange is going on here, this is not a "normal" stack overflow. There are a few things that stick out to me.

First, Hermes has a fairly large register stack, it usually allows deeper recursion than JSC or v8. You can run this example locally and see what it prints:

let depth = 0;
let printStack = false;

function foo() {
    ++depth;
    foo();
}
try {
    foo();
} catch (e) {
    if (printStack)
        print(e.stack);
    else
        print("depth =", depth, e);
}

When I run it on my Mac, I get 95319 recursive invocations, which is pretty deep (compared to 13997 for v8, 45615 for JSC).

The second thing to notice is that JS exceptions in Hermes always carry a stack trace, regardless of how it is built, even in production mode. If you change printStack to true in the above example, the trace will be printed. Even if you strip the debug information from the bytecode, etc, something will always be printed.

My third observation is that, AFAICT (I just performed a search), Hermes itself does not contain the string "no stack" from your error.

The last point is particularly interesting. Something in the integration must be intercepting the stack trace and erasing it.

As I understand it, you are running this in "developer mode", with Metro, fast refresh, etc? So, there are quite a few pieces of the puzzle there that are working together at runtime, which I actually don't understand well (I understand Hermes quite well, but not necessarily how it is integrated in React Native and Metro). We should do a search in React Native, Metro, etc, for something generating the string "no stack".

This is what I got so far, off the top of my head. After the holidays (on Tuesday), I will discuss it with others from the Hermes team to see whether they have ideas.

Thanks for being so helpful @tmikov, I appreciate it very much.

I'll spend some time looking into what you suggested, but wanted to share that I found a potential source of the "no stack" message in react-native JSI code ๐Ÿ‘€

Also, is it possible that there's a size limit being reached for a single bundle being evaluated by Hermes? This bundle I'm talking about is around 100mb

While it's likely the "no stack" message is coming from there, it's not clear why the stack is being discarded. Something else may be catching the exception and stripping some of the information. It would be interesting to poke at the exception and look at its type and prototype (if applicable), that may give some hint as to where it is coming from. (since it is missing the stack property)

You can get that value just by catching the exception in JS, or by using JSError::value().

Another thing to note is that there was actually a bug in our stack configuration that caused the stack limit to be artificially low, even when a larger stack limit is specified. This was fixed in 03f2dff. It is not yet in RN, but when it is, it will allow you to raise the stack limit using RuntimeConfig::Builder::withMaxNumRegisters. This bug is specific to using Hermes through JSI, which is why it does not show up when you use the hermes CLI tool directly.

@mikeduminy so, it seems like the RN+Hermes version you are using may be incorrectly configured with 16x smaller stack size. Are you able to build Hermes locally and patch one number? Change

static constexpr unsigned kMaxNumRegisters =
to just be = 1024 * 1024.

@tmikov That fixed it ๐Ÿ˜ฎ ๐Ÿ‘ ๐Ÿ™‡

@neildhar Thanks for the detail. Would you like us to dig deeper into the value of the error given that the change above fixed it?

Do you have any idea what version of react-native the fix (03f2dff) would be in?

@mikeduminy so, it seems like the RN+Hermes version you are using may be incorrectly configured with 16x smaller stack size. Are you able to build Hermes locally and patch one number? Change

static constexpr unsigned kMaxNumRegisters =

to just be = 1024 * 1024.

Are there any risks to shipping with this (16x) change? Is it possible that we could break things on lower-end devices?

@mikeduminy EDIT: didn't see the previous comment.

@mikeduminy This value consumes 8 MB virtual address space, which is only "converted" to RAM when actually used. So, in theory it shouldn't break things on any device. However it is probably more than you need, since I believe you were seeing the problem only in dev mode locally?

Have you tried finding the minimum value that works locally (using binary search)?

If I had to ship something in a hurry, I would use 512 * 1024.

@tmikov Gotcha ๐Ÿ‘

Yes, we're only seeing it locally in dev mode, but we'd like to use the same hermes build in both dev and prod to catch any problems in development. Also, on iOS swapping the code out between environments would result in changes to the pod lockfile which is annoying to have to ignore locally (because sometimes you might actually have cause to change this).

@mikeduminy

Would you like us to dig deeper into the value of the error

It's certainly still odd that the stack trace was deleted. If you do investigate further, I'd be curious to hear what you find. Especially since this may be a sign of a bug somewhere in Hermes/RN.

Do you have any idea what version of react-native the fix (03f2dff) would be in?

I expect it to be in 0.72. But I'll share this discussion with the RN team in case it makes sense to include it in a point release for 0.71 given that it is impacting users.

@neildhar @mikeduminy correct me if I am wrong, but 03f2dff doesn't actually fix this problem because it limits the stack size to 64 * 1024?

@tmikov It allows you to configure the stack size via the RuntimeConfig, so you would be able to adjust the heap size without maintaining a custom build of Hermes.

I'm not sure how conveniently RN surfaces the RuntimeConfig options to developers though (and therefore whether this is necessarily an improvement). Perhaps @mikeduminy knows how easy it would be to make this change.

@neildhar I believe we should change the default config to at least 256 * 1024 (pending confirmation from @mikeduminy that such a size works). I believe 512 * 1024 might be needed for parity with JSC, which, on my system, is able to make about 45,000 recursive calls (though it is possible that it is different on mobile).

Apologies for the delay ๐Ÿ™ My local environment was not set up for the testing iterations.

@tmikov It looks like 256 * 1024 works for us. I'm checking 128 and will update this message with the result.
EDIT: 128 works too! So does 64 actually ๐Ÿค”

@neildhar Try as I might I was unable to get more information from the error. If I have more time later I will try again.

@tmikov It allows you to configure the stack size via the RuntimeConfig, so you would be able to adjust the heap size without maintaining a custom build of Hermes.

I'm not sure how conveniently RN surfaces the RuntimeConfig options to developers though (and therefore whether this is necessarily an improvement). Perhaps @mikeduminy knows how easy it would be to make this change.

I don't know of any RuntimeConfig options being surfaced through RN but we could always apply local patches to RN to make this work. The first step is to make sure this ability makes its way to RN.

Ideally we'd have this change available in 0.70 (the version we are on), or 0.71 (the latest version), instead of 0.72 (the next version). I took a look at how RN builds and bundles Hermes and I'm not sure if it would be possible to easily do since hermes doesn't follow a tagged/branched version strategy, but I'm curious if you think it would be possible to backport this change to either of those versions?

I'm not sure if it would be possible to easily do since hermes doesn't follow a tagged/branched version strategy

There are tags on facebook/hermes that correspond to the version of Hermes bundled in RN. The tag is listed in node_modules/react-native/sdks/.hermesversion e.g. https://github.com/facebook/hermes/tree/hermes-2022-09-14-RNv0.70.1-2a6b111ab289b55d7b78b5fdf105f466ba270fd7. So Hermes maintainers could create a branch in facebook/hermes based on that tag, commit a patch and then use that commit in an upcoming 0.70.x release of RN.

Lots of coordination that needs to happen though.

Is a custom patch for previous RN versions something that the Hermes team is willing to do? @tmikov @neildhar

@mikeduminy we can probably release patches setting the correct default stack size: 128 or 256. I am saying "probably", since I am not familiar with the RN release process. cc: @neildhar who understand it much better.

@tmikov thanks, I want to first confirm something about the code before the change.

static constexpr unsigned kMaxNumRegisters =
(512 * 1024 - sizeof(::hermes::vm::Runtime) - 4096 * 8) /
sizeof(::hermes::vm::PinnedHermesValue);

Can you help me understand what the values of sizeof(::hermes::vm::Runtime) and sizeof(::hermes::vm::PinnedHermesValue) would be?

Earlier you mentioned that 1024*1024 was 16x more than we had before, which implies this value was 64*1024 before. However, in my testing, 64*1024 seems to work (32*1024 does not). I'd like to make sure that my results make sense before going further, as I could have made a mistake during testing.

@mikeduminy this is just a hack to calculate the size of the Hermes register stack that can fit with the runtime on a 512KB native thread stack. So we have the size of the native thread stack (512KB) minus the size of the runtime, minus 8 4KB pages of extra space, and we divide that by the size of a Hermes register, to get the available register stack size.

All of that is outdated and should be removed.

I don't actually know what it calculates to, I just assumed that it was 64K, but I may be way off.

128 * 1024 registers (which is 128 * 1024 * 8 bytes) seems reasonable to me.

I'd like to make sure that my results make sense before going further, as I could have made a mistake during testing

Technically it was slightly lower than 64 * 1024 because we subtracted some extra space as tmikov mentioned above. That extra space is likely just enough for your application to run.

custom patch for previous RN versions

Those tags are maintained and used by the RN team. If the RN team is planning further point releases for 0.70 and 0.71, the fix should port cleanly to them. cc @cortinico regarding backporting

Thanks for the heads up. We can totally backport Hermes changes inside RN (the infras was set up to allow for this).

If you're interested in having anything backported to a release series of RN, please comment here: https://github.com/reactwg/react-native-releases/discussions

(Provided 03f2dff is safe to cherry-pick on top of previous version of Hermes).

Anyway, as of today, this change is scheduled to land in the Bundled Hermes with React Native 0.72

I just want to remind everybody, again, that 03f2dff does not really fix this problem. We need another change on top of it, for the next RN release, and separate patches for previous releases.

I just want to remind everybody, again, that 03f2dff does not really fix this problem. We need another change on top of it, for the next RN release, and separate patches for previous releases.

So you are suggesting a bump to 128? I can make that change if everyone agrees to it.

Beyond that the integration into RN should probably expose a way in userland to adjust this limit. I see several points in RN where the RuntimeConfig is created and used - places where withMaxNumRegisters could be called to change the limit. That change is a bigger one and needs more specific knowledge about where to make the changes. So for now I vote for the proposed change to 128*1024 and backporting at least to 0.71, maybe to 0.70.

@mikeduminy I agree.

128*1024 should work nicely. It is only 1 MB of address space, and the actual RAM is consumed only if the stack gets that high, so we are not wasting memory.

In older versions we need to update kMaxNumRegisters, but in the current version (after 03f2dff), we need to update the default configuration size here: 03f2dff#diff-fe65dc57e9fc0262272f68b4e66424c75dfe8a4a2cf5d02bffdfc4aa8d9aefecR38 .

Then RN won't ideally need to call withMaxNumRegisters (it is probably a good idea to allow customization in some way, but that is an orthogonal problem to the default stack size being too low in Hermes).

PRs have been created to double the default in Hermes and apply the same default to the 0.71 and 0.70 tracking branches. I'm coordinating with the RN team via the 0.71.4 discussion and the 0.70.8 discussion. What would really help move these over the line is for the hermes team to review (and hopefully approve) the linked PRs.

If I've missed anything please let me know ๐Ÿ™

@mikeduminy thanks for making those. I've imported your PR to the main branch. Once that lands, I'll defer to the RN team on how to transplant it onto the release branches, the other PRs are probably not needed.

Happy to help, and thank you to all involved (especially @tmikov) for helping us diagnose and fix this edge case ๐Ÿ™

For the RN team, the PRs for 0.70 and 0.71 show the compatible change for those respective versions and are really what will make this change useful for us today.

kelset commented

thanks for making those. I've imported your PR to the main branch. Once that lands, I'll defer to the RN team on how to transplant it onto the release branches, the other PRs are probably not needed.

@neildhar I think we'll actually need to merge the PRs from @mikeduminy since the change for main is different from the ones for 0.71/0.70.

@kelset I think we could just pick both 03f2dff and #923 (once it is committed). That minimises divergence from trunk, and fixes the underlying bug in setting the register stack size via the RuntimeConfig, in the event that someone does want to build from source and specify withMaxNumRegisters.

kelset commented

@kelset Lorenzo Sciandra FTE I think we could just pick both 03f2dff and #923 (once it is committed). That minimises divergence from trunk, and fixes the underlying bug in setting the register stack size via the RuntimeConfig, in the event that someone does want to build from source and specify withMaxNumRegisters.

sounds good to me - in that case then the other two PRs from @mikeduminy should be closed.

Hi, I'm trying to upgrade to Hermes in our RN app and I'm getting the ERROR RangeError: Maximum call stack size exceeded (native stack depth), js engine: hermes. No big JSON files. I guess this is just Javascript code accessing the bridge too often?

Is the default max stack size going to be increased in RN 0.72?

@Waltari10 the fix has landed in RN 0.71.4 and will likely be cherry picked into RN 0.70.9. It will also be in RN 0.72.

@Waltari10 the fix has landed in RN 0.71.4 and will likely be cherry picked into RN 0.70.9. It will also be in RN 0.72.

Thanks for the info! Curiously we are already using 0.71.4 in our app though and still getting the error ๐Ÿค”

Disclaimer: I'm not an expert.

Oh I just noticed that your error message includes (native stack depth) which is a different error. The one that was fixed in RN 0.71.4 was a JS Register max stack size error, but yours is about the native stack size (which is much lower). You can find more information in this comment.

No big JSON files. I guess this is just Javascript code accessing the bridge too often?

Since this is a native stack issue I guess that neither JSON nor frequent bridge access is the problem. What does your stack trace look like?

Have a look at the stack trace in this comment. Notice the multiple lines that say: at apply (native). This indicates that a native call is being made. Note: this is not a react native call, but rather what the runtime considers a native call, which would be things like Function.prototype.call, Function.prototype.apply, or (maybe) Array.prototype.map.

Maybe the transpilation step of your builds is adding too many of these native calls to functions that are called in a deeply-nested way. You could see if any changes to your babel settings could fix it. Or perhaps you need to hunt down what changed in your codebase from when it was working.

This happened to me when I upgraded my react-native https://react-native-community.github.io/upgrade-helper/?from=0.63.4&to=0.64.0

 WARN  Require cycle: node_modules/core-js/internals/microtask.js -> node_modules/core-js/internals/microtask.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Blob.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/XMLHttpRequest.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 WARN  Require cycle: node_modules/rn-fetch-blob/index.js -> node_modules/rn-fetch-blob/polyfill/index.js -> node_modules/rn-fetch-blob/polyfill/Fetch.js -> node_modules/rn-fetch-blob/index.js

Require cycles are allowed, but can result in uninitialized values. Consider refactoring to remove the need for a cycle.
 ERROR  RangeError: Maximum call stack size exceeded, js engine: hermes
 LOG  Running "ny_app" with {"rootTag":1}
 ERROR  Invariant Violation: "ny_app" has not been registered. This can happen if:
* Metro (the local dev server) is run from the wrong folder. Check if Metro is running, stop it and restart it in the current project.
* A module failed to load due to an error and `AppRegistry.registerComponent` wasn't called., js engine: hermes

Happens both on android and ios.

Setting inlineRequires: false in metro.config.js fixed it for me

I am also getting this error in Android (iOS working fine), this happened suddenly.
It was working fine till yesterday.

inlineRequires: false => is already added in file

Any solution?

Screenshot_1683805028

Hi,

Been struck by this issue too when migrating from RN 0.69 to 0.70.9 and enabling hermes in the process.

Would this limitation of the call stack on Function.call() also be applicable to the usage of Array.prototype.reduce? As the implementation of the reduce function is calling the Callable::executeCall4 which in turn uses ScopedNativeCallFrame.

auto callRes = Callable::executeCall4(

CallResult<PseudoHandle<>> Callable::executeCall4(

We are essentially using AJV for JSON validation, and that library is using recursion within the call and reduce method.

Also noted that Function.apply() is also limited by this call stack.

ScopedNativeCallFrame newFrame{runtime, 0, *func, false, args.getArg(0)};

tmikov commented

FWIW, this has been addressed in the next version of Hermes, but unfortunately we do not yet have a timeline for release or for backporting.

Hi @tmikov,

Would you mind helping me to clarify some things I'm a bit confused about?

I just got hit by the RangeError: Maximum call stack size exceeded (native stack depth) issue while working on my app.

This seems to be different from the original issue of this thread that was fixed in #135 (comment)
I'm correct? fixing this issue will cover both cases or we should have another issue to track the "native stack depth" one?

In any case, my issue is that I'm using a library that needs to do recursive work where it uses apply/call functions multiple times.
I understand that the functions are converted to native calls and there is stack limit of 128. On certain situations I hit the limit and got the error.

It works fine if I use JavaScriptCore as the engine, I did a rough test in the stack limit seems to be around ~2738 when using JSC.

Is there a way to increase the limit when using Hermes or any other workaround?
Unfortunately, I can't change a 3rd party library that my app depends on.

Here is a repo with issue: https://github.com/vmalvaro/hermes-range-error

Thanks!

tmikov commented

@vmalvaro the limit can be increased by changing Hermes as described in this comment.

@tmikov thanks for the quick response, I'm a bit of a newbie with Hermes so I'd appreciate your guidance.

This #135 means that I have to do that change in hermes/include/hermes/VM/Runtime.h, then a custom build of Hermes + react native from source as described here https://hermesengine.dev/docs/react-native-integration?

@vmalvaro

custom build of Hermes + react native from source as described here

Those instructions are unfortunately no longer accurate for newer versions of RN, which have changed how Hermes is consumed. If you're on an RN version that is 0.69+, you can set an environment variable to point RN to the source directory where you want it to get Hermes from. See https://github.com/facebook/react-native/blob/fd9e295befcd8781190ec26a6a2fc4ef39fb1c15/packages/react-native/ReactAndroid/hermes-engine/build.gradle#L42.

screen

Get this issue when disabling js DEV mode

 System:
    OS: macOS 13.4
    CPU: (12) arm64 Apple M2 Pro
  Binaries:
    Node: 20.0.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 9.6.4 - /opt/homebrew/bin/npm
    Watchman: 2023.05.01.00 - /opt/homebrew/bin/watchman
  SDKs:
    iOS SDK:
      Platforms: DriverKit 22.4, iOS 16.4, macOS 13.3, tvOS 16.4, watchOS 9.4
    Android SDK:
      API Levels: 31, 33
      Build Tools: 30.0.2, 30.0.3, 31.0.0, 33.0.0, 33.0.2
      System Images: android-33 | Google APIs ARM 64 v8a
  IDEs:
    Android Studio: Flamingo 2022.2.1 Patch 1 Flamingo 2022.2.1 Patch 1
    Xcode: 14.3/14E222b - /usr/bin/xcodebuild
  npmPackages:
    react: 18.2.0 => 18.2.0 
    react-native: 0.71.8 => 0.71.8 
tmikov commented

@Villar74 is your problem related to loading JSON, which is the topic of this specific issue?

If not, and you believe there is a bug in Hermes, please create a new issue, preferably with more details, including source reproducing the problem. It is almost impossible to diagnose a problem from a screenshot of a stack trace of a compiled bundle.

tmikov commented

Finally, this long standing problem has been fixed in 3213794 and should be available in the next release of RN.

Hey tmikov could you please send this update in the coming patch as I needed this issue to be resolved in our codebase

tmikov commented

@Yashtoddle I don't understand what you mean. Send what where?

Sorry for not explaining the stuff clearly @tmikov actually we are using Hermes in our project as the engine and now we are facing the maximum call stack size exceded error in the android app and this error is now being resolved it would be great if you send 3213794
in the coming patch of the react native as we need this urgently in our project

tmikov commented

@Yashtoddle we don't really control which changes are picked into each patch releases. You also need to be more specific about the version where you want this.

In any case I think you can request a pick here: https://github.com/reactwg/react-native-releases/discussions .

This is not a trivial bugfix, it is several commits including 18aa617 .

@neildhar @cortinico what is the feasibility of picking this?

@Yashtoddle this will be included in the upcoming RN 0.73. The feasibility of picking this fix into an earlier release will likely depend heavily on which RN version you are currently on.