1352950 - Should we avoid consuming stack space while reporting over recursion?

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Tooru Fujisawa [:arai]]

separated from bug 1316416.

the bug is about over recursion inside Thunderbird, but it crashes with stack overflow while reporting over recursion in JS,
and the crash signature also appears in Firefox, so there should be more underlying issue around that, separated from the over recursion itself.

it looks like we could avoid the crash by skipping the CaptureStack call or something near there.

example crash report:
https://crash-stats.mozilla.com/report/index/e49fbf9b-7fc3-4024-8fa7-243512170301

Comment 1

[Tooru Fujisawa [:arai]]

fitzgen, do you have any opinion about the crash?

Comment 2

[lussnig]

Hi, the stack trace is important for fixing and to see where the underlying issue is caused.
I would say that there are two points that should be changed.
1) Do the stack overflow earlyer so that there is more stack available for dumping the method trace.
2) Detect the loop like in you example and do not print the look N times but instead add an line:
- Start of Stack Loop N times here
- Ende of Stack Loop N times here
this shorten the stackdump but gave the important hint how deep the recursion was.

Comment 3

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> fitzgen, do you have any opinion about the crash?

With too much recursion errors it is super important to capture a stack so that the developer knows _which_ calls are blowing the stack. Without that, their only option would be to manually comb through their entire code base and dependencies' code bases.

That said, there are potential trade offs. Usually its direct recursion (rather than mutually recursive functions) that is blowing the stack, and so the first couple frames is enough information. Perhaps we can only capture N frames (where N is small) if we are capturing a stack because of too much recursion?

Or perhaps we just need to reserve more of a buffer on the stack for ourselves by reporting too much recursion errors more eagerly?

Either way, we definitely should *not* be crashing ;)