Open Bug 1663090 Opened 4 years ago Updated 17 days ago

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

Categories

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

80 Branch
defect

Tracking

()

People

(Reporter: john.david.dalton, Unassigned)

References

Details

(Keywords: parity-chrome)

Attachments

(1 file)

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.

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

Component: Untriaged → DOM: Core & HTML
Product: Firefox → Core

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();
    }
Status: UNCONFIRMED → NEW
Ever confirmed: true

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.

Safari Technology Preview also stops at await.

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>)
Severity: -- → S3
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: