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

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

Closed
soootaleb opened this issue Dec 12, 2020 · 7 comments · Fixed by #8809 or #9619
Closed

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

soootaleb opened this issue Dec 12, 2020 · 7 comments · Fixed by #8809 or #9619
Assignees
Labels
bug Something isn't working correctly cli related to cli/ dir

Comments

@soootaleb
Copy link

soootaleb commented Dec 12, 2020

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

  • I use the serve function and a top level await to received requests (ugraded to websockets with acceptWebsocket)
  • On a new request, the worker forwards the payload to the main thread using self.postMessage
  • The main thread can worker.postMessage in order to send messages on the network (in the WebSocket)

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:/soootaleb/abcd/blob/master/src/net.worker.ts

@bartlomieju
Copy link
Member

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?

@bartlomieju bartlomieju added bug Something isn't working correctly cli related to cli/ dir labels Dec 12, 2020
@bartlomieju bartlomieju self-assigned this Dec 12, 2020
@soootaleb
Copy link
Author

@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:/soootaleb/abcd/tree/deno-issue-8735/issue-8735

@soootaleb
Copy link
Author

soootaleb commented Dec 17, 2020

@bartlomieju Hey !

My earlier, deno 1.4.0, working version, used [email protected], 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 [email protected] 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
Copy link
Member

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

@bartlomieju
Copy link
Member

@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
Copy link
Author

Hi @bartlomieju thanks a lot for your insights

  • I've removed the async syntax from my worker
  • I didn't mean sending JS object for methods, I just wanted to avoid the call of JSON methods

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
Copy link
Member

Not yet fixed: #8839

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly cli related to cli/ dir
Projects
None yet
2 participants