[Pkg-javascript-devel] Bug#1030284: nodejs: [arm64] RangeError: Maximum call stack size exceeded

[Thorsten Glaser]

Hi James,

(you might wish to Cc <${bugnumber}-submitter at bugs.debian.org> so they
actually get the reply…)

>Are you able to determine whether https://github.com/nodejs/node/issues/41163
>(and/or any of the guidance within that thread) seems relevant to this bug?

It appears so. I commented there, thank you for finding that link.

I guess there is even a… quick patch… to make from this. I admit
I’m very confused by that statement:

“if you set it too high, you risk crashes”

That can’t be right.

Searching through the nodejs source for where this stack size is
set, I see multiple time bombs for all architectures.

deps/v8/src/common/globals.h does set the default stack size to
864/984 KiB in order to achieve an about 1 MiB stack for JS plus
C++ parts combined.

I wonder if this shouldn’t be getrlimit(RLIMIT_STACK) - overhead,
and then define the per-architecture overhead in a suitable way.

That lower 1 MiB total limit seems to be for Windows. The lower
arm64 limit is caused by “allocating MacroAssembler takes 120 [KiB]”
but the total could still be raised I think… at least on unixoid
platforms other than WebView-on-Android. Since the location of these
defaults is in v8, it also applies for browsers and whatnot, but
nodejs could indeed inspect the current ulimit and set a better
default for at least nōn-Windows systems.

I’m not, unfortunately, in the position to provide a quick patch,
being a C developer, not CFrustFrust, and all that. I think that
InitializeNodeWithArgs in src/node.cc, which already has a call
to V8::SetFlagsFromString(NODE_V8_OPTIONS, …), is the likely place
for adding code (suitably platform-ifdef’d) that does:

- get the ulimit
- subtract some arch-specific overhead target
- check that that’s positive (or >= V8_DEFAULT_STACK_SIZE_KB even,
  that might be a good idea)
- if so, pass this as synthetic --stack-size (or --stack_size?) to
  v8, overriding its default but allowing for a later option given
  by the user’s argv[] to override _that_, again

Might need to adjust some tests, too :~


Good luck,
//mirabilos
-- 
<igli> exceptions: a truly awful implementation of quite a nice idea.
<igli> just about the worst way you could do something like that, afaic.
<igli> it's like anti-design.  <mirabilos> that too… may I quote you on that?
<igli> sure, tho i doubt anyone will listen ;)