search

denoland / deno

A modern runtime for JavaScript and TypeScript.
https://deno.com
MIT License
94.73k stars 5.26k forks source link

Workers & top-level-await changed behavior between 1.4.6 and 1.6.0 #8735

Closed soootaleb closed 3 years ago

soootaleb commented 3 years ago

Hello,

I've been implementing a distributed KV Store with Deno recently in version 1.4.6.

I use a worker for the network layer (so that the webserver is not blocking the main program):

This works perfectly in 1.4.6; I declare the self.onmessage function before starting the top level await, and the worker runs it when I worker.postMessage, even if the top level await is running as a web server.

But I tried to bump to Deno 1.6.0, and while the worker is still able to forward incoming messages to the main thread, it seems the self.onmessage handler is no longer called when the main thread posts a message to the worker.

I identified the top level await to be responsible since commenting it allows self.onmessage to be called correctly.

I've tried to explore the latest changes and issues, but I didn't find anything mentioning a change in behavior regarding the blocking / threading behaviors or workers / top level awaits.

Can anyone enlighten me ?

Regards

NB: The worker code is here https://github.com/soootaleb/abcd/blob/master/src/net.worker.ts

bartlomieju commented 3 years ago

I've tried to explore the latest changes and issues, but I didn't find anything mentioning a change in behavior regarding the blocking / threading behaviors or workers / top level awaits.

Can anyone enlighten me ?

@soootaleb in 1.5 we had a fix for top-level-await implementation that prevent some nasty edge cases. Unfortunately it looks like it had impact on workers implementation as well... 😬

Unfortunately linked code can't really be used as a test case, could you try to boil down reproduction?

soootaleb commented 3 years ago

@bartlomieju thanks a lot for your reactivity :)

Here we go: I created a subfolder with example code on a dedicated branch, see the README, reproducing is easy

https://github.com/soootaleb/abcd/tree/deno-issue-8735/issue-8735

soootaleb commented 3 years ago

@bartlomieju Hey !

My earlier, deno 1.4.0, working version, used std@0.65.0, so I just verified the problem didn't come from the http module (serve function) latest 0.80.0 version.

I can confirm it doesn't come from the http module since Deno 1.6.0 with std@0.65.0 doesn't work either

I hope you find some time to check that, It's a huge blocker for me and I think an awful bug for any software requiring a non-blocking web server (i.e any software which is not a web service)

Thank!

bartlomieju commented 3 years ago

@soootaleb I will look into this issue, but can't say when. I'll get back to you when I do.

bartlomieju commented 3 years ago

@soootaleb thanks for repro repo, I've created a test cased based on it in #8809 and confirmed there is an actual bug. It might take some time to fix it though as it requires to alter significant parts of WebWorker implementation.

Aside from that, your reproduction has a few quirks that prohibit it to work as expected:

soootaleb commented 3 years ago

Hi @bartlomieju thanks a lot for your insights

I can't wait to see the result, hope you'll notify me when progress is made. Until then, let me know if I can be of any help.

Regards

bartlomieju commented 3 years ago

Not yet fixed: https://github.com/denoland/deno/pull/8839