1663090 - Promises created from async functions don't resolve when document is detached.

Status: NEW

(Core :: DOM: Core & HTML, defect, P3)

Description

[John-David Dalton]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36

Steps to reproduce:

(async () => {
var i = document.createElement('iframe');
document.body.appendChild(i);
var a = i.contentWindow.eval('(async () => await 1)');
i.remove(); // someone removes the iframe
const v = await a();
console.log(v);
})();

Actual results:

Our use case has code that works when wired up and then someone removes the iframe and await no longer resolves.

Expected results:

The snippet works in Chrome.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Tooru Fujisawa [:arai]]

Thank you for reporting.

The issue is the following:

1. await 1 in the iframe's async function enqueues a promise reaction job to the job queue
2. in the next microtask checkpoint, the job is performed [1],
   but given that the global for the iframe is dying because it's detached,
   the callback isn't called [2]
3. await 1 waits forever, given the promise isn't resolved
4. the promise for the iframe's async function is left pending
5. await a() waits forever, given the promise isn't resolved

I'll check how the spec defines the steps for this case.

[1] https://searchfox.org/mozilla-central/rev/2fb77a351ab34e9994f01377e2e12486a50f1737/xpcom/base/CycleCollectedJSContext.cpp#585

bool CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool aForce) {
...
  for (;;) {
    RefPtr<MicroTaskRunnable> runnable;
...
      runnable = std::move(mPendingMicroTaskRunnables.front());
      mPendingMicroTaskRunnables.pop();
...

    if (runnable->Suppressed()) {
...
    } else {
      if (mPendingMicroTaskRunnables.empty() &&
          mDebuggerMicroTaskQueue.empty() && suppressed.empty()) {
        JS::JobQueueIsEmpty(Context());
      }
      didProcess = true;

      LogMicroTaskRunnable::Run log(runnable.get());
      runnable->Run(aso);
      runnable = nullptr;
    }
  }

[2] https://searchfox.org/mozilla-central/rev/2fb77a351ab34e9994f01377e2e12486a50f1737/xpcom/base/CycleCollectedJSContext.cpp#201-213

class PromiseJobRunnable final : public MicroTaskRunnable {
...
  virtual void Run(AutoSlowOperation& aAso) override {
    JSObject* callback = mCallback->CallbackPreserveColor();
    nsIGlobalObject* global = callback ? xpc::NativeGlobal(callback) : nullptr;
    if (global && !global->IsDying()) {
      // Propagate the user input event handling bit if needed.
      nsCOMPtr<nsPIDOMWindowInner> win = do_QueryInterface(global);
      RefPtr<Document> doc;
      if (win) {
        doc = win->GetExtantDoc();
      }
      AutoHandlingUserInputStatePusher userInpStatePusher(
          mPropagateUserInputEventHandling);

      mCallback->Call("promise callback");
      aAso.CheckForInterrupt();
    }

Comment 3

[Tooru Fujisawa [:arai]]

First, when an iframe gets removed, its document's browsing context is set to null.

https://html.spec.whatwg.org/#the-iframe-element

When an iframe element is removed from a document, the user agent must discard
the element's nested browsing context, if it is not null, and then set the
element's nested browsing context to null.

https://html.spec.whatwg.org/#a-browsing-context-is-discarded

To discard a browsing context browsingContext, run these steps:

  1. Discard all Document objects for all the entries in browsingContext's session history.
...

https://html.spec.whatwg.org/#discard-a-document

To discard a Document document:

...
  7. Set document's browsing context to null.

Then, HostEnqueuePromiseJob says it doesn't run the job if the document's "browsing context" is null.

https://html.spec.whatwg.org/#hostenqueuepromisejob

...
  6. Queue a microtask on the surrounding agent's event loop to perform the
     following steps:
    1. If job settings is not null, then check if we can run script with job
       settings. If this returns "do not run" then return.
...

https://html.spec.whatwg.org/#check-if-we-can-run-script

The steps to check if we can run script with an environment settings object
settings are as follows. They return either "run" or "do not run".

  1. If the global object specified by settings is a Window object whose
     Document object is not fully active, then return "do not run".
...

https://html.spec.whatwg.org/#fully-active

A Document d is said to be fully active when d's browsing context is non-null,
d's browsing context's active document is d, and either d's browsing context is
a top-level browsing context, or d's container document is fully active.

This means a promise reaction is ignored if the global is dying,
unless there's something else that cleans up pending promises when the global dies.

I'll look into the spec some more.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: testcase, with try-catch added to immediately log error case

Comment 5

[Tooru Fujisawa [:arai]]

Safari Technology Preview also stops at await.

Comment 6

[Tooru Fujisawa [:arai]]

fwiw, chrome behaves differently between local file and remote file.

with remote file (attachment), it always prints 1.

with local file (downloaded attachment), it prints 1 if DevTools is closed on load, and it throws the following error if DevTools is opened on load:

TypeError: Promise resolve or reject function is not callable
    at Promise.then (<anonymous>)

Comment 7

[johan]

Our clients ran into this issue when using our components on Salesforce, who in turn recommend transpiling async / await away for FF https://developer.salesforce.com/docs/platform/lwc/guide/security-lwsec-async.html?_ga=2.249124807.1359724086.1730723767-755359336.1698399851. Would be great if that was not needed 😅

Comment 8

[Emilio Cobos Álvarez (:emilio)]

But is it a bug? My read of comment 3 is that this is per spec, and WebKit behaves the same... So in any case this seems like a bug in Chromium?

Tooru, do you know if there's a crbug for this? Should we file it?

Comment 9

[Tooru Fujisawa [:arai]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #8)

But is it a bug? My read of comment 3 is that this is per spec, and WebKit behaves the same... So in any case this seems like a bug in Chromium?

Yes, this is per spec.

Tooru, do you know if there's a crbug for this? Should we file it?

I'm not aware of existing bug filed for chromium.
and yeah, it would be nice to file bugs on other engines, referring the whatwg issue

Comment 10

[Nige White]

Then let's suggest a spec change.

Infinite waits are just a bug.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

Well, executing JS of detached / inactive documents is arguably also a bug? Specially since rejecting the promise involves JS execution.

Comment 12

[Emilio Cobos Álvarez (:emilio)]

Also interesting is what spec change... https://github.com/whatwg/html/issues/2621 has a lot of discussion, some of which I'm not an expert on :)