RESOLVED FIXED 234190
WebCore::createDOMException() should abort early if termination is pending.
https://bugs.webkit.org/show_bug.cgi?id=234190
Summary WebCore::createDOMException() should abort early if termination is pending.
Mark Lam
Reported 2021-12-10 18:07:12 PST
Attempting to create Error objects may re-enter the VM, which we should not do when termination is pending.
Attachments
[fast-cq] proposed patch. (1.61 KB, patch)
2021-12-10 18:16 PST, Mark Lam
no flags
Mark Lam
Comment 1 2021-12-10 18:16:38 PST
Created attachment 446853 [details] [fast-cq] proposed patch.
Darin Adler
Comment 2 2021-12-10 18:29:14 PST
Comment on attachment 446853 [details] [fast-cq] proposed patch. View in context: https://bugs.webkit.org/attachment.cgi?id=446853&action=review > Source/WebCore/bindings/js/JSDOMExceptionHandling.cpp:145 > + VM& vm = lexicalGlobalObject->vm(); > + if (UNLIKELY(vm.hasPendingTerminationException())) > + return jsUndefined(); What about the other similar functions, like the ones called by this function? For example, createSyntaxError? I don’t think we need to put "vm" into a local variable, even though we do that often, since we are only using it once here.
Darin Adler
Comment 3 2021-12-10 18:30:19 PST
I suspect we’ll need this test for hasPendingTerminationException in more places. I don’t think this one function could possibly be the only one with a unique need for it.
Darin Adler
Comment 4 2021-12-10 18:31:34 PST
We should think about exactly which level is responsible for this check, and possibly move it elsewhere.
Mark Lam
Comment 5 2021-12-10 19:11:35 PST
I was hoping createDOMException() would be a good choke point, but I didn't do the due diligence. You are correct: the underlying factory methods are called from so many places in WebCore. On JSC side, we have regimented exception checks which would prevent these from being called. But on WebCore side, perhaps we need something more. I'll look into to moving the check lower, or see if I can think of a more elegant solution.
Mark Lam
Comment 6 2021-12-11 08:53:02 PST
On second thought, this fix works, and will prevent http/wpt/fetch/ tests from failing flakily due to this issue. So, let's land this first to help alleviate the bots while we think of better solutions.
EWS
Comment 7 2021-12-11 08:56:27 PST
Committed r286912 (245138@main): <https://commits.webkit.org/245138@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446853 [details].
Radar WebKit Bug Importer
Comment 8 2021-12-11 08:57:16 PST
Note You need to log in before you can comment on or make changes to this bug.