WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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
<
rdar://problem/86365930
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug