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>)