234190 2021-12-10 18:07:12 -0800 WebCore::createDOMException() should abort early if termination is pending. 2021-12-11 08:57:16 -0800 1 1 1 Unclassified WebKit WebCore JavaScript WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 mark.lam mark.lam cdumez darin webkit-bug-importer youennf oldest_to_newest 1823134 0 mark.lam 2021-12-10 18:07:12 -0800 Attempting to create Error objects may re-enter the VM, which we should not do when termination is pending. 1823136 1 446853 mark.lam 2021-12-10 18:16:38 -0800 Created attachment 446853 [fast-cq] proposed patch. 1823138 2 446853 darin 2021-12-10 18:29:14 -0800 Comment on attachment 446853 [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. 1823139 3 darin 2021-12-10 18:30:19 -0800 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. 1823142 4 darin 2021-12-10 18:31:34 -0800 We should think about exactly which level is responsible for this check, and possibly move it elsewhere. 1823150 5 mark.lam 2021-12-10 19:11:35 -0800 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. 1823216 6 mark.lam 2021-12-11 08:53:02 -0800 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. 1823219 7 ews-feeder 2021-12-11 08:56:27 -0800 Committed r286912 (245138@main): <https://commits.webkit.org/245138@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 446853. 1823221 8 webkit-bug-importer 2021-12-11 08:57:16 -0800 <rdar://problem/86365930> 446853 2021-12-10 18:16:38 -0800 2021-12-11 08:56:29 -0800 [fast-cq] proposed patch. bug-234190.patch text/plain 1645 mark.lam SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI4Njg5MykKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDIwIEBACisyMDIxLTEyLTEwICBNYXJrIExh bSAgPG1hcmsubGFtQGFwcGxlLmNvbT4KKworICAgICAgICBXZWJDb3JlOjpjcmVhdGVET01FeGNl cHRpb24oKSBzaG91bGQgYWJvcnQgZWFybHkgaWYgdGVybWluYXRpb24gaXMgcGVuZGluZy4KKyAg ICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzNDE5MAorCisg ICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIEF0dGVtcHRpbmcg dG8gY3JlYXRlIEVycm9yIG9iamVjdHMgbWF5IHJlLWVudGVyIHRoZSBWTSwgd2hpY2ggd2Ugc2hv dWxkIG5vdCBkbworICAgICAgICB3aGVuIHRlcm1pbmF0aW9uIGlzIHBlbmRpbmcuCisKKyAgICAg ICAgVGhpcyBpc3N1ZSBtYW5pZmVzdGVkIGFzIGFuIEFTU0VSVCBmYWlsdXJlLCBhbmQgd2FzIGRp c2NvdmVyZWQgd2hpbGUgcnVubmluZworICAgICAgICBodHRwL3dwdC9mZXRjaC8gbGF5b3V0IHRl c3RzIHdpdGggYSBEZWJ1ZyBidWlsZCBvbiBhbiBNMSBNYWMuICBJdCBhbHNvIG1hbmlmZXN0ZWQK KyAgICAgICAgb24gc29tZSB0ZXN0aW5nIGJvdHMuCisKKyAgICAgICAgKiBiaW5kaW5ncy9qcy9K U0RPTUV4Y2VwdGlvbkhhbmRsaW5nLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OmNyZWF0ZURPTUV4 Y2VwdGlvbik6CisKIDIwMjEtMTItMTAgIEVyaWMgQ2FybHNvbiAgPGVyaWMuY2FybHNvbkBhcHBs ZS5jb20+CiAKICAgICAgICAgW21hY09TXSBBZGQgbmV3IHNjcmVlbiBhbmQgd2luZG93IGNhcHR1 cmUgYmFja2VuZApJbmRleDogU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvSlNET01FeGNlcHRp b25IYW5kbGluZy5jcHAKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMv SlNET01FeGNlcHRpb25IYW5kbGluZy5jcHAJKHJldmlzaW9uIDI4NjY2MCkKKysrIFNvdXJjZS9X ZWJDb3JlL2JpbmRpbmdzL2pzL0pTRE9NRXhjZXB0aW9uSGFuZGxpbmcuY3BwCSh3b3JraW5nIGNv cHkpCkBAIC0xNDAsNiArMTQwLDEwIEBAIHZvaWQgcmVwb3J0Q3VycmVudEV4Y2VwdGlvbihKU0ds b2JhbE9iamUKIAogSlNWYWx1ZSBjcmVhdGVET01FeGNlcHRpb24oSlNHbG9iYWxPYmplY3QqIGxl eGljYWxHbG9iYWxPYmplY3QsIEV4Y2VwdGlvbkNvZGUgZWMsIGNvbnN0IFN0cmluZyYgbWVzc2Fn ZSkKIHsKKyAgICBWTSYgdm0gPSBsZXhpY2FsR2xvYmFsT2JqZWN0LT52bSgpOworICAgIGlmIChV TkxJS0VMWSh2bS5oYXNQZW5kaW5nVGVybWluYXRpb25FeGNlcHRpb24oKSkpCisgICAgICAgIHJl dHVybiBqc1VuZGVmaW5lZCgpOworCiAgICAgc3dpdGNoIChlYykgewogICAgIGNhc2UgRXhpc3Rp bmdFeGNlcHRpb25FcnJvcjoKICAgICAgICAgcmV0dXJuIGpzVW5kZWZpbmVkKCk7Cg==