Stream: wasmtime

Topic: Wasmtime preview2 `Sync`

Dave Bakker (badeend) (Jan 12 2024 at 20:20):

Hi! I was just wondering; why are there so many + Sync requirements in the WASI implementation?
As a test, I removed all the constraints and after refactoring one function in the filesystem implementation, the entire testsuite runs just fine

Alex Crichton (Jan 12 2024 at 20:41):

Some of them may have been added too eagerly, if they can be removed and everything still passes then I don't think there's any reason to keep them

Dave Bakker (badeend) (Jan 19 2024 at 19:26):

In an attempt to perform this change I hit a snag. As said before, the preview2 testsuite builds and run without problems. But the in the CLI code, specifically src/commands/run.rs, many wasmtime parts come together into one single Host type, where the + Sync is a requirement.

AFAICS, to make it work the Host type would need to be split up into a CoreHost (preview1_ctx, wasi_threads, ..) and a ComponentHost (preview2_ctx, preview2_table, wasi_http, ..). Does this sound about right?

Alex Crichton (Jan 19 2024 at 20:26):

Ah ok so maybe that's where this came from... yeah if splitting up like that works that seems reasonable

Alex Crichton (Jan 19 2024 at 20:26):

although I can see where that also might make organization pretty hairy

Dave Bakker (badeend) (Jan 19 2024 at 20:40):

image.png

How does this work exactly? I thought preview2 was componentmodel-only. But here (see arrow) it is enabled for non-components

Alex Crichton (Jan 19 2024 at 20:49):

it's the few lines above it, there's two implementations of preview1 and one goes through preview2 primitives

Alex Crichton (Jan 19 2024 at 20:49):

so that's where the implementation might get hairy

Dave Bakker (badeend) (Jan 19 2024 at 20:51):

Ah, so in this case, the guest module still sees only preview0/1, but internally wasmtime then uses the preview2 as their implementation, correct?

Alex Crichton (Jan 19 2024 at 20:53):

right yeah

Dave Bakker (badeend) (Jan 19 2024 at 20:53):

Hmm

Dave Bakker (badeend) (Jan 19 2024 at 21:13):

I can't find anything regarding this option in the docs. Is this an internal option that's used for testing the preview2 adapter only? Or is it for production use-cases too?

Dave Bakker (badeend) (Jan 19 2024 at 21:17):

If it's for testing purposes only I could probably just slap a Mutex around it and call it a day. If it's not, well then the road probably ends here.. ¯\_(ツ)_/¯ Which would be a shame, because other than these few lines of code, I've got it working just fine

Dave Bakker (badeend) (Jan 19 2024 at 21:45):

I could probably just slap a Mutex around it

Nope, that doesn't work :)

Alex Crichton (Jan 19 2024 at 22:02):

Wanna put up a PR and I can try to take a look?

Alex Crichton (Jan 19 2024 at 22:02):

(with the error message)

Dave Bakker (badeend) (Jan 19 2024 at 22:26):

Sure
https://github.com/badeend/wasmtime/commit/9fd350840e83ae103b2178516cec0510d3efb5a4
(You've got to ignore the enormous amount of code duplication for now :P )
The problem is on lines 953-970 where I can't return a reference to the contents of the mutex, because the LockGuard is dropped before the function returns.

Dave Bakker (badeend) (Jan 19 2024 at 22:27):

And the branch is: https://github.com/badeend/wasmtime/tree/no-sync

Alex Crichton (Jan 19 2024 at 22:32):

I won't be able to get to this today, but I'll take a crack at it next week

Dave Bakker (badeend) (Jan 20 2024 at 15:50):

After thinking about it some more, I abandoned the Core/ComponentHost refactor and ended up with a much simpler solution.

Dave Bakker (badeend) (Jan 20 2024 at 15:50):

https://github.com/bytecodealliance/wasmtime/pull/7802

Alex Crichton (Jan 22 2024 at 18:41):

sorry just got to this, but that PR looks great and I'm glad Pat beat me to the punch on review!

Joel Dice (Jan 22 2024 at 19:02):

I was just now wondering why e.g. ResourceTable::push put a Sync bound on T. So glad to see that removed. Thanks, @Dave Bakker (badeend) !

Last updated: Oct 23 2024 at 20:03 UTC