"The object has been freed and can't be used anymore"

[jayphelps]

FWIW I tried throwing this (via vanilla miniflare) at an large and complex project that uses Cloudflare Workers and many (if not all) of my usages induced the The object has been freed and can't be used anymore error. I made a minimal reproduction that induces it:

addEventListener('fetch', (event) => {
  const response = new HTMLRewriter()
    .on('example1', {
      element(element) {
        return new Promise((resolve) => {
          setTimeout(() => {
            element.after('something after example 1', {
              html: true,
            });
            resolve();
          });
        });
      },
    })
    .transform(new Response('<example1></example1>'));

  event.respondWith(response);
});

[jayphelps]

Forgot to mention that I built what's in master from source (to be sure I had the latest) then manually swapped it into miniflare (and confirmed it was being used.) Still, I may have made a mistake in doing so.

[mrbbot]

Hello! 👋 Thanks for raising this. I've just tried this locally and am able to reproduce the error. What's really strange is that I've got tests (for Miniflare) that do almost exactly the same thing that pass. 😕 I'll try investigate this tomorrow.

[mrbbot]

This was a fun one. 😅 I used dyn_ref from the JsCast trait to detect if a handler returned a Promise in Rust:

https://github.com/mrbbot/html-rewriter-wasm/blob/71c603515ba4c548e2da272aae96b59f95218ad5/src/handlers.rs#L38-L44

To check whether something was a Promise, wasm-bindgen generated the following code:

getObject(arg0) instanceof Promise

The problem with this is that Miniflare's Promise is in a different realm, because I don't pass it into the sandbox (even if I did, async functions would use the realm's Promise and fail). This meant Rust thought these were synchronous handlers so didn't pause execution, meaning when the Promises did finally resolve, everything had already been freed.

I fixed this by patching the Promise detection code to be:

(obj instanceof Promise) || (typeof obj === "object" && typeof obj.then === "function")

I've published this fix now as miniflare@1.3.2 (since it's a major issue). Could you try it out again and see if that fixes things?

[jayphelps]

Yep, that solved it! Thanks for the swift fix 🚀

There's another pattern some folks use to check "instanceof" cross-realm in case you hadn't seen it before: Object.prototype.toString.call(maybeAPromise) === '[object Promise]' though of course it's not foolproof cause someone could implement get [Symbol.toStringTag]() { return 'Promise'; } it's still "more accurate" than .then checks. Normally I might not mind .then checks since it allows folks to use Promise-like things that aren't actually Promises, but Cloudflare's real HTMLRewriter does not support this, so might be a good idea to match their behavior so folks don't incorrectly think that code would work in prod.

[mrbbot]

Awesome, glad that worked for you! 😃

Thanks for sharing that alternative promise detection code too, I'll switch to it in the next release.

[mrbbot]

Looks like for Node.js at least, util.types.isPromise works cross-realm too.

[jayphelps]

Nice. I had no idea that existed.

…

On Aug 22, 2021, at 7:20 AM, MrBBot ***@***.***> wrote:  Looks like for Node.js at least, util.types.isPromise works cross-realm too. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.