Spawn new task if we hit stack overflow #30419
Conversation
If we see the "Maximum call stack size exceeded" error we know we've hit stack overflow. We can recover from this by spawning a new task and trying again. Effectively a zero-cost trampoline in the normal case. If it errors again in the retryTask pass, the other error handling takes over which causes this to be able to still not infinitely stall.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
| switchContext(previousContext); | ||
| return; | ||
| } | ||
| if (x.message === 'Maximum call stack size exceeded') { |
There was a problem hiding this comment.
V8 and Hermes has the same message.
There was a problem hiding this comment.
Should we log some warning here? Is there any way product developers can action on this / any other downsides to having trees deep enough to hit this?
There was a problem hiding this comment.
I don't know if it's actionable. If you have a large site like facebook you're likely to have giant trees. However, you might also not have the ability to increase the stack limit of the service you're using.
|
@tom-sherman That might need a limit like the useEffect loop in Fiber but that's also already a concern with recursive async ones. |
|
I think it's not quite a severe DX regression with async components because it's more trivial to kill and restart (no state) and their async-ness means it's easier to attach a debugger between tasks. |
Only if they actually have I/O but if it's just an async function with nothing in it, it still stalls because microtasks don't yield. |
|
Well actually maybe our Fizz implementation current still yields in that case? I think maybe Flight doesn't anymore or something like that. EDIT: No it's the reverse. I don't think Fizz yields to macrotasks. |
|
Ah I just realised this PR is server 🤦 In that case, it's pretty bad to hang the server forever I think? 😅 In a serverless environment it's bad also because you'll get a timeout and not even get a stack trace in some cases. |
|
Yea but regardless that's already something you can do. It's a halting problem - not a stack overflow problem. |
If we see the "Maximum call stack size exceeded" error we know we've hit stack overflow. We can recover from this by spawning a new task and trying again. Effectively a zero-cost trampoline in the normal case. The new task will have a clean stack. If you have a lot of siblings at the same depth that hits the limit you can end up hitting this once for each sibling but within that new sibling you're unlikely to hit this again. So it's not too expensive. If it errors again in the retryTask pass, the other error handling takes over which causes this to be able to still not infinitely stall. E.g. when the component itself throws an error like this. It's still better to increase the stack limit for performance if you have a really deep tree but it doesn't really hurt to be able to recover since it's zero cost when it doesn't happen. We could do the same thing for Flight. Those trees don't tend to be as deep but could happen. DiffTrain build for [96aca5f](96aca5f)
If we see the "Maximum call stack size exceeded" error we know we've hit stack overflow. We can recover from this by spawning a new task and trying again. Effectively a zero-cost trampoline in the normal case. The new task will have a clean stack. If you have a lot of siblings at the same depth that hits the limit you can end up hitting this once for each sibling but within that new sibling you're unlikely to hit this again. So it's not too expensive.
If it errors again in the retryTask pass, the other error handling takes over which causes this to be able to still not infinitely stall. E.g. when the component itself throws an error like this.
It's still better to increase the stack limit for performance if you have a really deep tree but it doesn't really hurt to be able to recover since it's zero cost when it doesn't happen.
We could do the same thing for Flight. Those trees don't tend to be as deep but could happen.