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

LibJS: Handle call stack limit exceptions in NewPromiseReactionJob #3450

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

trflynn89
Copy link
Contributor

The promise job's fulfillment / rejection handlers may push an execution
context onto the VM, which will throw an internal error if our ad-hoc
call stack size limit has been reached. Thus, we cannot blindly VERIFY
that the result of invoking these handlers is non-abrupt.

This patch will propagate any internal error forward, and retains the
condition that any other error type is not thrown.

Fixes #3449

(The first couple commits are to remove the last users of MUST_OR_THROW_OOM so that it may be repurposed for use in the actual fix).

This was preventing clangd in my environment from processing the
Intrinsics class.
There are now no users of the MUST_OR_THROW_OOM macro. Let's rename this
macro to indicate it may be used to propagate any internal error (such
as the call stack limit error) in places that would otherwise crash due
to a MUST/VERIFY invocation.

Note there's no actual functional change here, as we weren't able to
ensure the internal error was an OOM error previously.
The promise job's fulfillment / rejection handlers may push an execution
context onto the VM, which will throw an internal error if our ad-hoc
call stack size limit has been reached. Thus, we cannot blindly VERIFY
that the result of invoking these handlers is non-abrupt.

This patch will propagate any internal error forward, and retains the
condition that any other error type is not thrown.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LibJS: Crash in PromiseJobs.cpp (stack exhaustion?)
2 participants