Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression in import starting in Deno 1.32.4 #19903

Closed
johnspurlock opened this issue Jul 21, 2023 · 14 comments · Fixed by #20006
Closed

Regression in import starting in Deno 1.32.4 #19903

johnspurlock opened this issue Jul 21, 2023 · 14 comments · Fixed by #20006
Assignees

Comments

@johnspurlock
Copy link
Contributor

Starting in 1.32.4, top level await inside imported module code cause the import call to hang, but curiously only when a Worker is running (even if unrelated!).

Took me forever to track this down - hopefully the Deno team can take a look, since it's been broken now for a few releases. Thanks!

Repro code:

    const workerCode = `
        console.log('worker!');
    `;
    const worker = new Worker(URL.createObjectURL(new Blob([ workerCode ])), { type: 'module' });
   
    const moduleCode = `
    console.log('module start');
    const hash = await crypto.subtle.digest('SHA-1', new TextEncoder().encode('data'));
    const __default = {};
    export { __default as default };
    console.log('module finish');
    `;
    console.log('before import ' + Deno.version.deno);
    await import(URL.createObjectURL(new Blob([ moduleCode ])));
    console.log('after import');
   
    worker.terminate();

output on Deno 1.32.3 and below (expected)

before import 1.32.3
worker!
module start
module finish
after import
<process exits>

output on Deno 1.32.4 and above (import never returns even though module executes)

before import 1.32.4
worker!
module start
module finish
<process hangs>
@bartlomieju
Copy link
Member

Seems similar to #19455. We'll look into it.

@bartlomieju bartlomieju self-assigned this Jul 22, 2023
@bartlomieju
Copy link
Member

@johnspurlock this is exactly the same problem as #19455. We are looking into it. Let's continue in that issue.

@mmastrac
Copy link
Contributor

Fixed in 1.35.3

@johnspurlock
Copy link
Contributor Author

Ah ok, thanks for taking a look

@johnspurlock
Copy link
Contributor Author

1.35.3 fixes the small repro case I posted, but unfortunately doesn't fix the real-world use case that prompted me to come up with it... perhaps I'll have to come up with another more complicated repro.

The real-world use case (hosting a particular denoflare example worker locally in a worker isolate) can be reproduced by running:

$ deno run -A --unstable https://raw.githubusercontent.com/skymethod/denoflare/6cdf6fc51e6bf550cd78796a4bacba0651189010/cli/cli.ts serve https://raw.githubusercontent.com/skymethod/denoflare/be474b3c8f9c9b5ed6d653ee904c2f6367fd0b07/examples/webtail-worker/webtail_worker.ts

You'll see:

Compiling https://raw.githubusercontent.com/skymethod/denoflare/6cdf6fc51e6bf550cd78796a4bacba0651189010/cli-webworker/worker.ts into worker contents...
Bundled https://raw.githubusercontent.com/skymethod/denoflare/6cdf6fc51e6bf550cd78796a4bacba0651189010/cli-webworker/worker.ts (process) in 361ms
runScript: https://raw.githubusercontent.com/skymethod/denoflare/be474b3c8f9c9b5ed6d653ee904c2f6367fd0b07/examples/webtail-worker/webtail_worker.ts
Bundled https://raw.githubusercontent.com/skymethod/denoflare/be474b3c8f9c9b5ed6d653ee904c2f6367fd0b07/examples/webtail-worker/webtail_worker.ts (process) in 37ms
worker: start

and then it will hang, not completing startup. A normal startup will have these two following lines:

Started in 659ms (isolation=isolate)
Local server running on http://localhost:8080

It works on Deno 1.32.3, but not subsequent versions, including 1.35.3.

This worker does a hash calculation as a top-level await in the module. I'm pointing to a particular revision since I've worked around it in the script itself in master (deferring the hash calc to first access), but I'd still like to get to the bottom of this, as user scripts should feel free to do stuff like this during module init - and it's not something I can work around in the denoflare runtime.

@bartlomieju bartlomieju reopened this Jul 27, 2023
@bartlomieju
Copy link
Member

Thanks for the repro @johnspurlock, we'll look into it.

@johnspurlock
Copy link
Contributor Author

Thanks! One perhaps relevant difference architecturally from the initial repro case is that denoflare sends the script to the webworker and calls import inside the webworker, which then hangs the webworker.

So imagine a more complicated workerCode that includes doing that dynamic import there instead of outside.

@bartlomieju
Copy link
Member

@johnspurlock any chance you could trim down the reproduction? I've been banging my head for a couple hours now to try and come up with more minimal reproduction but so far, everything I tried passes fine. I'm trying to create a regression test for this problem and bringing whole denoflare codebase for an integration tests is not really an option here.

@johnspurlock
Copy link
Contributor Author

Ok I will try

@johnspurlock
Copy link
Contributor Author

Alright try this:

    const moduleCode = `
    console.log('module start');
    const hash = await crypto.subtle.digest('SHA-1', new TextEncoder().encode('data'));
    const __default = {};
    export { __default as default };
    console.log('module finish');
    `;

    const workerCode = `
        console.log('worker!');

        globalThis.onmessage = (msg) => {
            const { moduleCode } = msg.data;
            (async () => {
                console.log('before import ' + Deno.version.deno);
                await import(URL.createObjectURL(new Blob([ moduleCode ])));
                console.log('after import');
                self.postMessage('thanks');
            })();
        }
    `;
    const worker = new Worker(URL.createObjectURL(new Blob([ workerCode ])), { type: 'module' });
    worker.onmessage = () => {
        console.log('worker.terminate');
        worker.terminate();
    }
    worker.postMessage({ moduleCode });

on 1.32.3

worker!
before import 1.32.3
module start
module finish
after import
worker.terminate
<process exit>

on 1.32.4 and 1.35.3

worker!
before import 1.3x.x
module start
module finish
<process hang>

@bartlomieju
Copy link
Member

Thanks you @johnspurlock, I can reproduce it now!

@bartlomieju
Copy link
Member

#20006 should fix it and the fix will be available in v1.36.0 later this week. Thanks for catching this @johnspurlock!

@johnspurlock
Copy link
Contributor Author

Awesome, thanks for getting to the bottom of this!

bartlomieju added a commit to denoland/deno_core that referenced this issue Aug 1, 2023
This fixes regressions reported in denoland/deno#19903.

We mistakenly were not waking up the event loop again if ops made progress
and there are pending dynamic imports waiting for TLA to resolve.
@johnspurlock
Copy link
Contributor Author

Confirmed the real-world scenario that prompted this issue is fixed when running on Deno 1.36.0. 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants