[webkit-reviews] review denied: [Bug 197172] JSC should have public API for unhandled promise rejections : [Attachment 376833] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 21 11:26:14 PDT 2019


Keith Miller <keith_miller at apple.com> has denied Ross Kirsling
<ross.kirsling at sony.com>'s request for review:
Bug 197172: JSC should have public API for unhandled promise rejections
https://bugs.webkit.org/show_bug.cgi?id=197172

Attachment 376833: Patch

https://bugs.webkit.org/attachment.cgi?id=376833&action=review




--- Comment #36 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 376833
  --> https://bugs.webkit.org/attachment.cgi?id=376833
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=376833&action=review

> Source/JavaScriptCore/runtime/JSGlobalObjectFunctions.cpp:772
> +    if (promise->inherits<JSInternalPromise>(vm))

We normally do this via jsDynamicCast<JSInternalPromise>(vm, promise). We have
optimizations for some cases there that we haven't applied to inherits and it
makes it easier to search for.

> Source/JavaScriptCore/runtime/VM.h:1072
> +    Vector<Strong<JSPromise>> m_aboutToBeNotifiedRejectedPromises;

I feel like this is going to be a huge list for non-trivial async programs. if
you have a loop with:

let p = new Promise((resolve, reject) => {
    ...
    reject(p)
});

p will be kept alive until the the microtasks are drained. Which means programs
will OOM if they never drain all their microtasks...

I think, at least, you'll need to clear handled promises at the GC flip. See
where we call finalizeUnconditionalFinalizers.


More information about the webkit-reviews mailing list