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

Spawn Workers always from root #19

Open
syrusakbary opened this issue Dec 16, 2021 · 15 comments
Open

Spawn Workers always from root #19

syrusakbary opened this issue Dec 16, 2021 · 15 comments

Comments

@syrusakbary
Copy link
Member

Safari 15.2 adds support for SharedArrayBuffers, which makes it "kind of" work with an old version of ate.
However, I tried tokera.sh with the new Safari and it breaks because of the usage of nested webworkers.

Screen Shot 2021-12-16 at 5 59 47 PM

WebWorkers are also supported in Safari, but it doesn't support nested WebWorkers (a Worker can't create another Worker. See Webkit issue).

If we are able to create webworkers from the root document instead of the inner workers, it would make Safari happy and 100% compatible with the shell (including having the shell fully working on iOS)

@john-sharratt
Copy link
Collaborator

Ah - i see - this could be possible but not sure of the implications with all the shared goodness going on with the terminal itself (e.g. terminal file system, stdout, wasm-bus, etc...) - the core of the terminal uses a-lot of synchronization primitives and message passing that's all in rust at the moment but maybe it will still work.

Let me take a look at it and see whats possible... iOS support would be a big win.

@john-sharratt
Copy link
Collaborator

The discrepancy on the old and new version version of tokera.sh is that the new version uses a rebuilt worker pool which fixed a load of bugs which were causing sub processes to freeze up. One of the things I also changed was that I made it so the pool would "pre-seed" a bunch of workers to reduce the latency when you run commands. That's why you get the message earlier in the new version but if you tried to do anything that spawns a thread on the old version you would hit the same error.

I think its better to do it like this anyway as at least the user knows it doesn't work sooner rather than later :-)

Looking at the code it should be possible to get the javascript to launch the worker outside of the context of the worker thread bindgen call which may resolve this. Might try to just put it in a super short timer and see if that does the job; in theory it should also mean that the same SharedArray goes to the worker and everything still works but we'll find out soon enough when I try it...

Let me give it go and see what I can do.

@john-sharratt
Copy link
Collaborator

Modified the thread pool so that it launches all web workers from the root thread - this should in theory fix it for Safari - I (shamefully) don't have a Safari browser to test as I have no iPhone's or Macs - I should probably spin up a browser testing service for this but that will have to be for another day.

Can you test it in the browser and see if its fixed?

@syrusakbary
Copy link
Member Author

I just tried in macOS + Safari, and it doesn't work for remote commands like ls

@john-sharratt
Copy link
Collaborator

Ok. Ill sign up for a online browser testing service and debug this one. Thanks!

@john-sharratt
Copy link
Collaborator

Well that sucks.... I signed up for browser testing suite and they only have 15.1
(https://app.lambdatest.com/)

What do you guys use over at wasmer.io .... or do you just test on real hardware?

Cheers

@syrusakbary
Copy link
Member Author

We test on real hardware (I use normally macOS for development).
Safari 15.2 was just released, so that's why probably the browser test suite doesn't have them.

Would having real hardware help for this? Happy to send a mac mini your way if that may help!

@john-sharratt
Copy link
Collaborator

Lets park it for a few days and see if a testing suite adds support soon - I think its better to make a fully automated regression test on tokera.sh so that many browsers can be tested on commits (CI/CD). Will look into what options there are for this in GitHub then get back to this thread.

@john-sharratt
Copy link
Collaborator

I am still checking back and forth on this one with the online browser testing suite I use but they still have not added 15.2.
Will keep looking for it but if you know of one that supports it happen to switch providers.

@syrusakbary
Copy link
Member Author

It seems Saucelabs supports it: https://saucelabs.com/platform/supported-browsers-devices

@john-sharratt
Copy link
Collaborator

excellent - I'll take a look at this.

@john-sharratt
Copy link
Collaborator

Firefox is now working.... I had to implement a cors proxy to the WAPM CDN because its not putting the allow cors headers on OPTIONS requests but I made it as minimal as possible - it tries the CDN first and only if the CORS headers are missing does it use the CORS proxy.

@john-sharratt
Copy link
Collaborator

Next up is Safari...

@john-sharratt
Copy link
Collaborator

Saucelabs does indeed support 15.... but not 15.2.... :-(

I did have another look if I could find one though and it does seem that 15.2 is supported here...
https://testingbot.com/

Only problem is its so dawn slow I can barely test it... I will put in a few more hours trying online testing suites before I move onto something else to code (at least I got firefox going) but its looking more and more like getting hold of a physical Mac is the only way to debug this properly.

Cheers
Cheers

@john-sharratt
Copy link
Collaborator

I was able to reproduce the error message you had on https://testingbot.com and unfortunately the work I did to try and get workers not spawning from workers hasn't panned out - I think the only solution from here is to move the thread pool totally out of web assembly and re-code it all pure javascript side spawning from the HTML side. It would then be possible to hook it back into the workers and allow for the terminal to spin up processes.

That's a significant amount of work and my javascript coding skills (or lack of) will slow this down so for now we will have to park this until some more help comes on hand to tackle this one.

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

No branches or pull requests

2 participants