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
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
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?
Ah ok so maybe that's where this came from... yeah if splitting up like that works that seems reasonable
although I can see where that also might make organization pretty hairy
How does this work exactly? I thought preview2 was componentmodel-only. But here (see arrow) it is enabled for non-components
it's the few lines above it, there's two implementations of preview1 and one goes through preview2 primitives
so that's where the implementation might get hairy
Ah, so in this case, the guest module still sees only preview0/1, but internally wasmtime then uses the preview2 as their implementation, correct?
right yeah
Hmm
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?
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
I could probably just slap a Mutex around it
Nope, that doesn't work :)
Wanna put up a PR and I can try to take a look?
(with the error message)
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.
And the branch is: https://github.com/badeend/wasmtime/tree/no-sync
I won't be able to get to this today, but I'll take a crack at it next week
After thinking about it some more, I abandoned the Core/ComponentHost refactor and ended up with a much simpler solution.
https://github.com/bytecodealliance/wasmtime/pull/7802
sorry just got to this, but that PR looks great and I'm glad Pat beat me to the punch on review!
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: Dec 23 2024 at 14:03 UTC