Andrew Brown (May 04 2023 at 15:44): Andrew Brown (May 04 2023 at 15:44):After getting approval for adding wasm32-wasi-threads as a tier-2 target to rustc (see #574), it was my intent earlier in the year to implement the new target and then fill in the missing std::thread::spawn implementation using the threads-compatible build of wasi-libc. (Rust's wasm32-wasi target already uses wasi-libc so using it again for wasm32-wasi-threads seems to make sense). I got sidetracked on other work, but want to help finish this work up if possible. @daxpedda contacted me the other day about this and volunteered to take a look and just yesterday @Georgii Rylov did the same. I propose we use this thread to split this up into smaller chunks and coordinate who does what.
Andrew Brown (May 04 2023 at 15:47):I think the most helpful thing I can do is document my progress on this before I was forced to stop. I had the wasm32-wasi-threads target building but compiling programs required linker flags (--export-memory I think?) that were only available in LLVM 16; as I waited for rustc to switch over to LLVM 16 (it now has), I got sidetracked. So here is a description of the steps I took to get that far: https://gist.github.com/abrown/9a94aaa164ad1a8d29d5e86b737bbdc6.
Andrew Brown (May 04 2023 at 15:49):@daxpedda, @Georgii Rylov, let me know what you think! Any others that want to help out feel free to jump in. @daxpedda, it would probably be helpful to know how much further you have progressed from the gist I sent? (No pressure! I just mean for coordination-sake...)
Georgii Rylov (May 04 2023 at 16:59):Great, thank you @Andrew Brown for gist and creating the channel for coordination
daxpedda (May 04 2023 at 17:04):I have not taken a look at it at all yet, I was planning to start next week.
daxpedda (May 04 2023 at 17:05):Would like to take on thread-local variables as I've already spent the time figuring out what Rust does there. But that's in the future right now.
daxpedda (May 04 2023 at 17:06):@Georgii Rylov I'm happy to do the Rust library part if you take on the "getting it to build" part, if you like to split it that way.
Georgii Rylov (May 04 2023 at 17:10):It might work yeah, let me take a look at it first
Georgii Rylov (May 11 2023 at 21:32):took a quick look at the guide, for me a new target(just a duplicate of wasm32-wasi made by changing the same files from gist) compiles code to wasm but even a hello world segfaults immediately
@Andrew Brown how did you understand what files to modify? did you check https://github.com/rust-lang/rust/commit/ace71240d233e71fcc6e3824fa2e5e05697fd2cc or an another earlier similar commit?
Andrew Brown (May 11 2023 at 22:29):Yes, commits like that one, conversations with others, some banging of my head on the wall, etc. It took me a while to even understand how to build the compiler the right way. What do you mean by "segfault"? I don't think that rustc should segfault as it tries to compile Rust code to the new wasm32-wasi-threads target (?!?). And if it does produce any WebAssembly, running that WebAssembly should not segfault either.
Andrew Brown (May 11 2023 at 22:29):What is the error you are seeing?
Georgii Rylov (May 18 2023 at 14:47):Seems I did something wrong first time, now it didn't segfault and adding a new target(not a thread one) seems work fine. Sorry for disappearing, I accidentaly wiped my boot partition so delayed getting back to it
Georgii Rylov (May 19 2023 at 11:48):@Andrew Brown Why did you wait for llvm 16 in rust btw?
Georgii Rylov (May 19 2023 at 11:51):It just feels realted to the problem with build that I'm trying to overcome now
"= note: rust-lld: error: --shared-memory is disallowed by std-532b41f7ca59a61b.19qx9c2akto1mw8b.rcgu.o because it was not compiled with 'atomics' or 'bulk-memory' features."
according to what I found I need to add options like "options.features = "+atomics,+bulk-memory,+mutable-globals".into();"
But it doesn't compile with "+atomics" and I suppose it might be related to usage of LLVM 16 https://github.com/rust-lang/rust/blob/master/compiler/rustc_codegen_llvm/src/llvm_util.rs#L413
bjorn3 (May 19 2023 at 14:49):You can use RUSTFLAGS="-Ctarget-features=+atomics,+bulk-memory,+mutable-globals" for as long as you use cargo build -Zbuild-std=std,panic_abort. Without the -Zbuild-std a version of the standard library without those features will be reused, which doesn't work.
Andrew Brown (May 19 2023 at 15:20):Georgii Rylov said:
Andrew Brown Why did you wait for llvm 16 in rust btw?
The reason was related to linker flags: prior to LLVM 16, --import-memory and --export-memory were not allowed together. The reason both are needed is an artifact of how WASI currently does things; see https://github.com/WebAssembly/WASI/issues/502 for more details.
Andrew Brown (May 19 2023 at 15:27):It sounds like the core of the problem boils down to "the linker wants all of the object files to have the same Wasm features." And some object from std has not been built this way. @bjorn3 is right that Cargo's way to rebuild std with all the right features is -Zbuild-std but I'm not completely sure for what you're doing that the command will look exactly like that (e.g., you might need to set those target features somewhere in the rustc code based on the target, like if target == wasm32-wasi, then target-features = ...). You're venturing into ground that I have not explored, though, so it may be more helpful if you can share the code or the command you're running?
Andrew Brown (May 19 2023 at 15:31):Just guessing: you're trying to do something like this but for some reason that does not affect how std is built?
Georgii Rylov (May 19 2023 at 19:05):yeah, that's where I picked the idea
Actually it seems I found a solution to build problems, now it builds and runs hello_world.wasm as expected
And when I try to run code that spawns a thread it fails as expected
thread 'main' panicked at 'failed to spawn thread: Error { kind: Unsupported, message: "operation not supported on this platform" }' and it's coming from here https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/thread.rs#L17
Georgii Rylov (May 19 2023 at 19:06):that's code that does the build https://github.com/rust-lang/rust/compare/master...g0djan:rust:godjan/wasi-threads
Georgii Rylov (May 19 2023 at 19:08):so I think @daxpedda already can start trying on top of that
I started trying myself, but I'm more at stage of reading the proposal and looking through code for other platforms
daxpedda (May 19 2023 at 19:37):alright, gonna start soon!
thanks for all the work so far!
Georgii Rylov (May 22 2023 at 12:23):@Andrew Brown My understanding is that you implemented pthread_create in wasi-libc and std::thread:spawn should do the same thing from rust. Therefore I suppose the rust implementation should call __wasI_thread_spawn as you did in wasi-libc and not supposed to try to call pthread_create from wasi-libc, right?
So could you explain a bit why is it needed to set wasi_root and whether wasi-libc is supposed to be used from rust anyhow?
And I assume that __wasi_thread_spwan should be exposed in a similar way as it's done in atomics
thank you for the answer about llvm btw
bjorn3 (May 22 2023 at 12:33):Rust uses wasi-libc for everything. I'm pretty sure that if libstd were to call __wasi_thread_spawn directly as opposed to going through wasi-libc's pthread_create impl that would mess up the thread local storage for both wasi-libc and libstd.
Georgii Rylov (May 22 2023 at 12:58):Okay, thank you
So supporting wasi-threads in rust would mean then export pthread_create, call it from std::thread::spawn and overcome all the build and tests problems around it or something like that?
Ramon Klass (May 22 2023 at 12:58):I like what Zulip did to : thread :
bjorn3 (May 22 2023 at 14:31):Georgii Rylov said:
Okay, thank you
So supporting wasi-threads in rust would mean then export pthread_create, call it from std::thread::spawn and overcome all the build and tests problems around it or something like that?
I think so.
Andrew Brown (May 22 2023 at 17:07):(I'm now online!) I think the "just use pthread_create" approach is the best bet for now. Though technically one _could_ use __wasi_thread_spawn directly I would bet there would be more issues to fix than just using pthread_create. Plus, since std also uses pthread_create for non-Wasm targets (here), there is already precedent for this. It gives us one place to fix things (i.e., wasi-libc) as we test things out in these early days and makes it more likely that objects built by rustc and clang won't have weird incompatibilities.
Andrew Brown (May 22 2023 at 17:10):@Georgii Rylov, also: congratulations on getting things building!
Georgii Rylov (May 22 2023 at 20:21):Thank you =)
Georgii Rylov (May 22 2023 at 20:21):Good, pthread_create sounds reasonable
daxpedda (May 25 2023 at 21:39):@Georgii Rylov
I reserved this weekend to work on this. So if you have anything you wanna share feel free to drop it here or commit it to your fork/branch.
Enrico Loparco (Jun 01 2023 at 13:14):@daxpedda any updates there?
daxpedda (Jun 01 2023 at 13:16):I mostly made some progress understanding what has been done and how I'm going to proceed.
Had a bunch of issues linking, but I got that resolved.
Gonna continue again this weekend.
Georgii Rylov (Jun 06 2023 at 16:56):@daxpedda Hey, sorry for not responding, branch that I shared is still up to date. Do you have any branch where I can follow your progress? I just got myself free to get back to work on wasi-threads
daxpedda (Jun 06 2023 at 17:00):No problem!
Unfortunately I wasn't able to continue last weekend, so nothing really to share. The weekend before that I just got things to link correctly.
daxpedda (Jun 06 2023 at 17:00):I plan to continue this weekend again, feel free to drop an update when you have something.
Georgii Rylov (Jun 06 2023 at 17:00):@Andrew Brown do you understand why some symbols are needed to be specified in extern "C" https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/os.rs#L22
and some others https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/os.rs#L64 can be used without specifying in extern
even both symbols are present in defined-symbols.txt
Georgii Rylov (Jun 06 2023 at 17:01):Alright I will keep it updated in this channel and will push things to my branch if I get any progress
daxpedda (Jun 06 2023 at 17:03):Georgii Rylov said:
Andrew Brown do you understand why some symbols are needed to be specified in extern "C" https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/os.rs#L22
and some others https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/os.rs#L64 can be used without specifying in extern
Isn't it just exported through here: https://github.com/rust-lang/rust/blob/3572d7451d1062e8bccf310af6bbf6255091d296/library/std/src/sys/wasi/os.rs#L19?
daxpedda (Jun 06 2023 at 17:04):If you follow it you land here: https://github.com/rust-lang/libc/blob/f171596308a616876d518b3491af716bd1629608/src/wasi.rs#L677
Georgii Rylov (Jun 06 2023 at 17:04):yep, but why strerror_r is just exported but getcwd isn't. Both symbols are defined in wasi-libc
Georgii Rylov (Jun 06 2023 at 17:04):Nice, that's what I was looking for. Thank you!
Andrew Brown (Jun 06 2023 at 17:18):Ok, it sounds like you guys figured it out but I'll comment anyways: I think getcwd is now exported by the libc crate (see here) but probably was not when that code was written. See the "add a few symbols not in upstream libc just yet" comment up above on line 17. But I bet the __wasilibc_get_environ function is never going to be available in libc (like, it will only be available in wasi-libc) so that has to be manually declared as extern "C" so that this code will link to it.
Andrew Brown (Jun 06 2023 at 17:19):I think we would need to do the same extern "C" for any special WASI functions that aren't going to be present in the upstream libc crate (e.g., the thread spawn and entry point)
bjorn3 (Jun 06 2023 at 20:22):If __wasilibc_get_environ is stable and won't be removed or changed in the future, then adding it to the libc crate may make sense. It already has a lot of platform specific api's exported by the platform's libc implementation.
Dan Gohman (Jun 06 2023 at 20:24):It's used by Rust, so it's basically stable and won't be removed any time soon.
Georgii Rylov (Jun 09 2023 at 09:34):Good news I spawned the first thread, it executed the code passed to the thread but ofc it panicked immediately after that :)
@daxpedda I updated the branch https://github.com/rust-lang/rust/compare/master...g0djan:rust:godjan/wasi-threads?expand=1
Georgii Rylov (Jun 09 2023 at 16:04):ran into this error
cannot recursively acquire mutex', library/std/src/sys/wasi/../unsupported/locks/mutex.rs:20:9
seems primitives were not actually implemented for wasi or wasm yet and this stub was used but now the stub is not suitable
according to https://github.com/rust-lang/rust/blob/master/library/std/src/sys/wasi/mod.rs#L32 at least locks mod needs to be implemented, but I presume some of these(or all) will be needed too
*#[path = "../unsupported/thread_local_dtor.rs"]
pub mod thread_local_dtor;
#[path = "../unsupported/thread_local_key.rs"]
pub mod thread_local_key;
#[path = "../unsupported/thread_parking.rs"]*
Georgii Rylov (Jun 13 2023 at 13:42):Seems implementing locks/pthread_mutex.rs was enough
Ran this
fn main() {
std::env::set_var("RUST_BACKTRACE", "1");
let mut join = vec![];
for i in 0..10 {
join.push(std::thread::spawn(move || {
println!("threadken {}", i);
}));
}
for j in join {
j.join().unwrap();
}
}
All threads were spawned, printed its number and the execution finished with 0 exit code.
Georgii Rylov (Jun 13 2023 at 13:44):I still get some random error when I launch things, but it seems to work correct even with this error
Failed to call function with name __main_void. Error: Exception: unreachable
Limited backtrace available:
I use wamr lib, probably will need to do a bit more work to make it run on wasmtime
Georgii Rylov (Jun 15 2023 at 09:46):Hi @Andrew Brown @daxpedda
Got several questions
1) Is there a ready testsuite for threads in rust?
2) To make threads actually useful threads at least some primitives are necessary.
- Should this work be done in a separate PR? Is a proposal Issue needed for it?
- What primitives would be needed first - mutex, condvar, rwlock ?
3) Are wasm/wasi targets tested anyhow at the moment? It requires using a wasm runtime like wasmtime or wamr but I haven't found traces of those in the rust repo
daxpedda (Jun 15 2023 at 10:28):1) I'm not aware of one.
2) I don't think this is actually relevant for the threads proposals, that's more part of the atomics proposals. I know it's already implemented and working for Wasm, but I don't know what the status of this is in WASI.
3) I don't know if they are tested in the Rust repo, but you would definitely some runtime unless you are just testing the binary output.
Andrew Brown (Jun 16 2023 at 15:36):For (1) it might be useful to take a look at the tests people have contributed to the wasi-threads repository. They can't be used directly (they are designed to test the engine) but you could clone the ideas into new Rust-based tests.
Andrew Brown (Jun 16 2023 at 15:38):For (2) it was my impression that @Alex Crichton had implemented most of this in the Rust standard library using the atomic instructions from the WebAssembly threads proposal.
Andrew Brown (Jun 16 2023 at 15:39):I would just test (3) separately for now and we can consult with other Rust contributors about this once a PR is created with the basic spawn functionality.
Georgii Rylov (Jun 20 2023 at 10:12):(2) Okay, I found Alex added it in the PR and later Mara added "futex" calls and replaced Alex implementation with using existing ones relying on "futex" calls here
Georgii Rylov (Jun 20 2023 at 10:13):Thank you
Georgii Rylov (Jun 21 2023 at 14:33):On wasmtime I'm was getting "2: missing required memory export" , @Andrew Brown do you know that anyone managed to at least compile some C code with WASI-threads and run it on wasmtime?
I would expect it works on wasi-threads proposal tests, but it seems it doesn't actually https://github.com/WebAssembly/wasi-threads/issues/36#issuecomment-1600945096
Georgii Rylov (Jun 21 2023 at 15:01):Found https://bytecodealliance.org/articles/wasi-threads , this example runs
Andrew Brown (Jun 21 2023 at 15:51):Yeah, I was going to point you to that last link; it definitely runs. I think what you are missing with "required memory export" is exporting the memory... wasi-threads wants the memory to be imported but the wasi-common crate expects it to be exported as well.
Andrew Brown (Jun 21 2023 at 15:53):As for the tests, I'm not exactly sure what is going on there. I know I've had some issues running those tests in the past; maybe a max memory size is needed or a configured stack size, like @Marcin Kolny mentioned in the thread.
Georgii Rylov (Jun 21 2023 at 16:29):With tests I checked that neither max-memory or stack-size helps https://github.com/WebAssembly/wasi-threads/issues/36#issuecomment-1601156113
Georgii Rylov (Jun 21 2023 at 16:31):But with Rust I'm exporting it https://github.com/rust-lang/rust/compare/master...g0djan:rust:godjan/wasi-threads?expand=1#diff-6f5295f4c3305d9c6bb8bdaa253b497a35dc3c05efb0f4d319ca82582ddf8391R13
@Marcin Kolny guess is that -pthread should help, but I don't see yet where to add it cause now I get when I add it to pre_link_args
rustc +stage1 -v --target=wasm32-wasi-threads -pthread /tmp/main.rs
error: Unrecognized option: 'p'
Marcin Kolny (Jun 21 2023 at 17:57):I don't think -pthread flag is supported by rustc. But you can look at the flags that are added to the compiler when -pthread is enabled and try adding the same to rustc command.
Georgii Rylov (Jun 22 2023 at 15:49):The diff in options with -pthread
```➜ rust git:(godjan/wasi-threads) ✗ diff without_pthread with_pthread
18a19,26
-target-feature
+atomics
-target-feature
+bulk-memory
-target-feature
+mutable-globals
-target-feature
+sign-ext
41a50
-pthread
45c54
< /var/folders/gx/qqscz_s14pb4bybmb705ngdc0000gr/T/thread_spawn-simple-7622fa.o
/var/folders/gx/qqscz_s14pb4bybmb705ngdc0000gr/T/thread_spawn-simple-fde3aa.o```
I added this options(besides -pthread), but didn't help
rustc seems to use rust-lld and it doesn't know --export-memory option
@Andrew Brown does rustc use wasm-ld too or rust-lld solely?
bjorn3 (Jun 22 2023 at 15:57):wasm-ld is lld. Lld can act like different linkers depending on how it is called. For example ld.lld is a unix liker, while lld-link.exe is a windows linker.
bjorn3 (Jun 22 2023 at 15:59):Rustc doesn't support the -pthread argument. Every rustc target either doesn't support threads or always supports it. The wasm32-wasi-threads target should unconditionally support threads unless something is wrong in it's implementation.
Georgii Rylov (Jun 22 2023 at 16:00):wasm-ld is lld
Okay, and rust-lld is just another lld?
bjorn3 (Jun 22 2023 at 18:27):Yes. I believe it only has a different name to prevent conflicts between the lld version that is part of rustc and an lld version that is globally installed.
Georgii Rylov (Jun 23 2023 at 17:07):rustup update helped, I needed a fresh version of rust-lld that also supports --export-memory flag.
Alex's comment helped https://github.com/rust-lang/rust/pull/112922#discussion_r1240001844
Georgii Rylov (Jun 23 2023 at 17:10):Also I opened PR https://github.com/rust-lang/rust/pull/112922
Alex Crichton (Jun 23 2023 at 17:15):Looking at the lld that rustc is using, it should have --export-memory? Can you share what you've been doing which causes lld to print an unexpected argument?
Alex Crichton (Jun 23 2023 at 17:15):namely export_memory is present in src/llvm-project/lld/wasm/Options.td
Georgii Rylov (Jun 23 2023 at 23:09):Can you share what you've been doing which causes lld to print an unexpected argument?
It seemed I was using rust-lld from stable and I didn't run rustup update for some time so it didn't support the flag at the time, now I added --export-memory as you suggested and it works after I ran rustup update
Georgii Rylov (Jun 23 2023 at 23:11):How are such changes to rust(compiled to wasm/wasi) or wasi-libc get tested if there's no runtime used in CI? It seems there are no tests in this PR for example https://github.com/WebAssembly/wasi-libc/commit/241060c3427c2cdd63aa1e0f7a804ab3f75a413f
Georgii Rylov (Jul 11 2023 at 13:00):@Andrew Brown @bjorn3 now I manually add these lines to config.toml to compile rustc
[target.wasm32-wasi]
wasi-root = "/opt/wasi-sdk/share/wasi-sysroot"
[target.wasm32-wasi-threads]
wasi-root = "/opt/wasi-sdk/share/wasi-sysroot"
I would assume that I need add downloading wasi-sdk and this lines somewhere in src/bootstrap/defaults so it actually gets to the compiler. But wasm32-wasi is in stable already and when I try to run ./x.py build --stage 1 --target wasm32-wasi on master branch it also requires to add the path to wasi-sysroot to config.toml so I'm not sure whether I need to add path to wasi-sysroot anywhere
bjorn3 (Jul 11 2023 at 13:04):When the wasm32-wasi target gets build in rust's CI wasi-root is set by CI. You only need to set wasi-root yourself if you are building it yourself.
bjorn3 (Jul 11 2023 at 13:05): Georgii Rylov (Jul 11 2023 at 13:08):then I probably need to add a similar lines in CI for wasm32-wasi-threads
Georgii Rylov (Jul 11 2023 at 13:08):thank you
Georgii Rylov (Jul 11 2023 at 13:51):@bjorn3 do you know how to upload llvm 16 to ci-mirrors.rust-lang.org ? LLVM 16 is needed for wasi threads and wasi toolchain in CI is using LLVM 15 now
It seems to need to be updated the same way as here https://github.com/rust-lang/rust/commit/e8af822423498e1393d71b9fa64f64b6182c5b44 but I don't know how to upload https://github.com/llvm/llvm-project/releases/download/llvmorg-16.0.4/clang+llvm-16.0.4-x86_64-linux-gnu-ubuntu-22.04.tar.xz to ci-mirrors
bjorn3 (Jul 11 2023 at 13:53):Someone on the infra team has to upload it.
Georgii Rylov (Jul 11 2023 at 13:56):Thanks, I will try to reach out to them
Andrew Brown (Jul 12 2023 at 01:13):Georgii Rylov said:
then I probably need to add a similar lines in CI for wasm32-wasi-threads
I think you might also need to build the wasm32-wasi-threads sysroot somehow, like this script does for wasm32-wasi: https://github.com/rust-lang/rust/blob/63ef74b6aa0448cee884324a1f47dde284176895/src/ci/docker/host-x86_64/dist-various-2/build-wasi-toolchain.sh
Georgii Rylov (Jul 12 2023 at 09:20):Yes, I also think so and already asked infra team in the Rust zulip to upload it to ci-mirrors https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Upload.20LLVM.2016.20to.20ci-mirrors.2Erust-lang.2Eorg/near/374279147
Georgii Rylov (Jul 12 2023 at 09:59):to upload LLVM 16 *
Georgii Rylov (Jul 12 2023 at 15:31):added it https://github.com/rust-lang/rust/pull/112922/commits/b3597038d1ec3e196ff4e9a7cf9132a1396f0343, though Idk how to test whether it's what needed to be done for ci
Georgii Rylov (Jul 19 2023 at 14:44):Per policies https://doc.rust-lang.org/nightly/rustc/target-tier-policy.html#tier-3-target-policy the new target must have maintainers.
I will add myself but wonder whether anybody volunteers too? It will make it easier for the target to get to Tier-2 as there's a requirement that there should be at least 2 maintainers
Georgii Rylov (Jul 19 2023 at 14:44):**A tier 2 target must have a designated team of developers (the "target maintainers") available to consult on target-specific build-breaking issues, or if necessary to develop target-specific language or library implementation details. This team must have at least 2 developers.
The target maintainers should not only fix target-specific issues, but should use any such issue as an opportunity to educate the Rust community about portability to their target, and enhance documentation of the target.**
Those are the responsibilities when it gets to Tier-2
Georgii Rylov (Jul 19 2023 at 14:46):@Andrew Brown @Alex Crichton @daxpedda @Marcin Kolny @Enrico Loparco
Alex Crichton (Jul 19 2023 at 14:48):I'm happy to volunteer yeah, so feel free to list my name
Andrew Brown (Jul 19 2023 at 15:01):You can add me as well
Georgii Rylov (Jul 19 2023 at 15:19):Nice will do, thank you guys
Marcin Kolny (Jul 19 2023 at 16:18):Happy to be added too
Enrico Loparco (Oct 06 2023 at 00:23):Hello, I started playing around and noticed that the join is hanging in case I compile without optimizations
use std::sync::mpsc;
use std::thread;
fn main() {
let (send, recv) = mpsc::channel();
let thread = thread::spawn(move || {
let v = vec![1, 2, 3];
send.send(v).unwrap();
});
recv.recv().unwrap();
thread.join().unwrap();
}
Tried both WAMR and Wasmtime.
rustc -v --target=wasm32-wasi-preview1-threads test.rs
./iwasm test.wasm # It hangs...
wasmtime --wasm-features=threads --wasi-modules=experimental-wasi-threads test.wasm # It hangs...
Everything is fine if I compile with rustc -v --target=wasm32-wasi-preview1-threads -O test.rs.
Any ideas on what I can check? @Alex Crichton @Andrew Brown
Alex Crichton (Oct 06 2023 at 13:47):Alas I'm not sure myself, that would likely require a good deal of runtime debugging as well as debugging the wasm itself.
bjorn3 (Oct 06 2023 at 13:52):Would the core dump functionality of wasmtime help here? Or is there no way to core dump a hang?
Alex Crichton (Oct 06 2023 at 13:54):Yeah unfortunately core dumps only happen on traps right now. You can perhaps try out fuel/epochs but those don't interact well with threads right now because the spawned store won't inherit the same configuration options at this time.
Andrew Brown (Oct 06 2023 at 18:19):My first thought is that there could be something wrong here: https://github.com/rust-lang/rust/blob/64fa0c34d7cb1a2d522414ab2c87024e465bd613/library/std/src/sys/wasm/atomics/futex.rs. @Alex Crichton, I've been hunting around in the library/std/src/sync/mpsc and library/std/src/sync/mpmc directories to see if we bottom out in a futex somehow but gave up. Is that the case somehow?
Alex Crichton (Oct 06 2023 at 18:22):I think it bottoms out in mutexes/condvars nowadays, but I'm not up to date really on the implementation of mpsc
Andrew Brown (Oct 06 2023 at 18:29):I guess I'm just wondering if there is some wait that isn't getting it's appropriate notify... if there were a way to list the waiting waits when a wasi-thread exits then we could post a log warning or something to help troubleshooting
Alex Crichton (Oct 06 2023 at 18:58):@Andrew Brown not that I'm aware of right now
Andrew Brown (Oct 11 2023 at 18:48):@Enrico Loparco did you end up figuring this issue out? If not, maybe this PR could help: https://github.com/bytecodealliance/wasmtime/pull/7220.
Enrico Loparco (Oct 12 2023 at 10:08):Hello, I started playing around and noticed that the join is hanging in case I compile without optimizations
Looking again, the hanging is caused by .recv(), not the join as I said initially. And it seems to happen when recv is called before send in the example I posted above.
Taking a closer look and then I'll try https://github.com/bytecodealliance/wasmtime/pull/7220.
Enrico Loparco (Oct 12 2023 at 12:29):Naive question, how do I build wasmtime with wasi threads enabled? I used curl https://wasmtime.dev/install.sh -sSf | bash in the past, but if I build from the repo (to get your commit) I get
./target/debug/wasmtime --wasm-features=threads --wasi-modules=experimental-wasi-threads /Users/eloparco/dev/forks/wasm-micro-runtime/samples/wasi-threads/build/test.wasm
error: unexpected argument '--wasm-features' found
tip: a similar argument exists: '--wasm'
Usage: wasmtime <--optimize <KEY[=VAL[,..]]>|--codegen <KEY[=VAL[,..]]>|--debug <KEY[=VAL[,..]]>|--wasm <KEY[=VAL[,..]]>|--wasi <KEY[=VAL[,..]]>> <WASM>...
For more information, try '--help'.
Enrico Loparco said:
Naive question, how do I build wasmtime with wasi threads enabled? I used
curl https://wasmtime.dev/install.sh -sSf | bashin the past, but if I build from the repo (to get your commit) I get./target/debug/wasmtime --wasm-features=threads --wasi-modules=experimental-wasi-threads /Users/eloparco/dev/forks/wasm-micro-runtime/samples/wasi-threads/build/test.wasm error: unexpected argument '--wasm-features' found tip: a similar argument exists: '--wasm' Usage: wasmtime <--optimize <KEY[=VAL[,..]]>|--codegen <KEY[=VAL[,..]]>|--debug <KEY[=VAL[,..]]>|--wasm <KEY[=VAL[,..]]>|--wasi <KEY[=VAL[,..]]>> <WASM>... For more information, try '--help'.
Same exact problem... I'm looking for help as well...
/mnt/e/Code/WebAssemblyExperiments/wasmthread/bin$ /mnt/e/Code/wasmtime/target/release/wasmtime --wasm-features=threads --wasi-modules=experimental-wasi-threads ./t
hreads.wasm
error: unexpected argument '--wasm-features' found
tip: a similar argument exists: '--wasm'
Usage: wasmtime <--optimize <KEY[=VAL[,..]]>|--codegen <KEY[=VAL[,..]]>|--debug <KEY[=VAL[,..]]>|--wasm <KEY[=VAL[,..]]>|--wasi <KEY[=VAL[,..]]>> <WASM>...
For more information, try '--help'.
Support for wasi-threads is on-by-default, but the CLI flags have changed so nowadays that looks like --wasm threads --wasi threads, or -W threads -S threads
Callum Macmillan (Oct 31 2023 at 13:54):@Andrew Brown Do I need different flags to enable those wait and notify logs? I picked this up from Enrico and executed the example with
RUST_LOG=wasi_common=trace wasmtime --wasm threads --wasi threads -D logging=y ...
I don't see any wait or notify logs, but I get some WASI related logs eg TRACE wasi_common::snapshots::preview_1::wasi_snapshot_preview1 > wiggle abi; module="wasi_snapshot_preview1" function="fd_write"
I opened a ticket for the mpsc channels issue here https://github.com/rust-lang/rust/issues/117440
I think I was able to get a trace from WAMR debugger that points towards a signal from allocation, through channel send. I created a repo for quick reproduction https://github.com/cimacmillan/WasiThreadsSchedYieldBug. Something interesting is that commenting out the thread join, and replacing with arbitrary thread sleep resolved the issue. Not sure how to dive further past those traces.
Do you have any guidance on how I can trace it further / anything else I could provide that would help root-cause?
Andrew Brown (Oct 31 2023 at 14:34):I don't know what version of Wasmtime you're on, but if it's recent you may want to try WASMTIME_LOG=wasmtime_runtime=trace .... Someone changed the environment variable for logging recently and I think the log statements you're looking for are in the runtime crate. What happens when you use that?
Callum Macmillan (Oct 31 2023 at 15:12):@Andrew Brown Thanks, I built wasmtime on commit 1d46c2d589d97cd5a13c17c88c8d288f643bdaff and getting this output running this example on debug, before it hangs
2023-10-31T15:10:33.642456Z TRACE wasmtime_runtime::memory: memory.atomic.notify(addr=0x105910, count=4294967295)
receiving
2023-10-31T15:10:33.642793Z TRACE wasmtime_runtime::memory: memory.atomic.wait32(addr=0x105958, expected=4294967295, timeout=None)
sending [1, 2, 3]
Then on release build it outputs
2023-10-31T15:09:17.319447Z TRACE wasmtime_runtime::memory: memory.atomic.notify(addr=0x104d78, count=4294967295)
receiving
2023-10-31T15:09:17.319740Z TRACE wasmtime_runtime::memory: memory.atomic.wait32(addr=0x104db8, expected=4294967295, timeout=None)
sending [1, 2, 3]
2023-10-31T15:09:17.319958Z TRACE wasmtime_runtime::memory: memory.atomic.notify(addr=0x104db8, count=1)
sent
2023-10-31T15:09:17.319986Z TRACE wasmtime_runtime::memory: memory.atomic.notify(addr=0x105fc0, count=1)
2023-10-31T15:09:17.319992Z TRACE wasmtime_runtime::memory: memory.atomic.notify(addr=0x104d2c, count=1)
received [1, 2, 3]
Cool, at least you should be able to see which pairs aren't matching up. I think the other thing that might help you troubleshoot is wasi-backtrace: I'm planning to propose that as a new API this Thursday at the WASI meeting. It will dump a Wasm back trace when called; I found this very, very helpful when trying to debug a recent deadlock.
Callum Macmillan (Nov 03 2023 at 16:37):@Andrew Brown Thanks Andrew, using those it's easy to see that the atomic waits & notifies don't match up. I added some similar logs to WAMR as well as backtraces on each notify, just as I'm a bit more familiar with that VM. I added some pastes to https://github.com/rust-lang/rust/issues/117440#issuecomment-1792750313. The main thread is stuck on wasi futex wait, and it looks like spawned thread isn't sending emitting a notify after sending vec over the channel. Annoyingly, when I add some debug logging to std lib, it changes the program behaviour / fails for a different reason. Would appreciate any ideas / tips you might have of where to look. Otherwise, will keep digging.
Alex Crichton (Nov 03 2023 at 16:55):@Callum Macmillan reading over that issue (sorry drive-by comment), have you ruled out stack overflow?
Alex Crichton (Nov 03 2023 at 16:55):most issues I've seen with wasi threads are that the shadow memory stack overflows in debug mode
Alex Crichton (Nov 03 2023 at 16:56):and that silently corrupts everything
Callum Macmillan (Nov 06 2023 at 16:38):Thanks @Alex Crichton , @Marcin Kolny had linked me this change to set WASM global to the stack limit. I added some quick checking in WAMR and see the global stack pointer overflows only on the debug build in the child thread in the mpmc list channel start send. Then increasing DEFAULT_MIN_STACK_SIZE in std thread for wasi I no longer get the error.
Callum Macmillan (Nov 07 2023 at 15:39):@Alex Crichton what do you think about increasing the default minimum stack size for wasi threads in Rust, at least for debug builds (here)? Had discussed with Marcin that we can make changes to detect overflow at runtime with LLVM (issue) so we can exit cleanly rather than deadlock. Then for the wasi mpsc channels issue the open question is if anything needs to change in rust std lib. I'd presume the default spawned thread stack size should allow that example when compiled in debug mode (and the suite of thread tests). I was also wondering whether it's possible to determine it with static analysis, so that the default thread stack size could be determined at compile time depending on build type / depth of source. In the meantime, we can set the env variable so we're unblocked.
Alex Crichton (Nov 07 2023 at 16:15):I definitely agree that the best fix here is to add stack overflow detction through llvm, and I think it'd make sense to just increase the default stack size. Even in debug mode it shouldn't be so easy to overflow the stack
Alex Crichton (Nov 07 2023 at 16:16):In that I think it makes sense to increase, and increasing only in debug mode sounds ok, but I think it would be ok to always have it increased too
Georgii Rylov (Nov 15 2023 at 11:32):@Andrew Brown @Alex Crichton is there a reason why max-memory is set in .wasm binary?
I want to add an api/setting to set max_memory value on vm_creation which will overwrite max-memory of the binary and use it instead
Usecase: on-call can overwrite it manually in a config in a special circumstances instead of recompiling and maintaining a binary for every different number of max_memory
Georgii Rylov (Nov 15 2023 at 13:03):to WAMR*
Alex Crichton (Nov 15 2023 at 15:04):WebAssembly validation of shared memories requires a maximum to be specified, so it's a spec-level thing. Runtimes can of course end up having a maximum smaller than this, but runtimes cannot arbitrarily raise this.
Georgii Rylov (Nov 16 2023 at 11:17):Got it, thank you
so it's a spec-level thing
Do you know where can I find these specs?
Runtimes can of course end up having a maximum smaller than this
Okay, @Enrico Loparco made a point that it can be useful too. When there're low-end devices that can't allocate that much memory
Alex Crichton (Nov 16 2023 at 16:01):The threads spec is located at https://webassembly.github.io/threads/
Enrico Loparco (Feb 29 2024 at 15:50):WebAssembly validation of shared memories requires a maximum to be specified, so it's a spec-level thing. Runtimes can of course end up having a maximum smaller than this, but runtimes cannot arbitrarily raise this.
Do you think it would make sense to have that mechanism already in the runtime (without adding an API)? When using shared memory and trying to allocate linear memory, if the allocation (or mmap) fails, we could try allocating half of it (or have a callback registered by the user to decide how to decrease it).
When using shared memory in production, having a fixed max memory value for all targeted devices is quite a limitation and shipping different wasm files (where the only difference is the max memory value) depending on the specific device is quite unfeasible. @Wenyong Huang
Wenyong Huang (Mar 01 2024 at 00:49):Enrico Loparco said:
WebAssembly validation of shared memories requires a maximum to be specified, so it's a spec-level thing. Runtimes can of course end up having a maximum smaller than this, but runtimes cannot arbitrarily raise this.
Do you think it would make sense to have that mechanism already in the runtime (without adding an API)? When using shared memory and trying to allocate linear memory, if the allocation (or mmap) fails, we could try allocating half of it (or have a callback registered by the user to decide how to decrease it).
When using shared memory in production, having a fixed max memory value for all targeted devices is quite a limitation and shipping different wasm files (where the only difference is the max memory value) depending on the specific device is quite unfeasible. Wenyong Huang
Yes, it should make sense, for shared memory, if allocating memory fails when instantiation, we can decrease the size and allocate again, allocating half of max memory seems a little small, maybe we can decrease one or two pages until allocation succeeds, or as you said, register a callback. Another method may be adding another API like wasm_module_inst_t wasm_runtime_instantiate_ex(module, args, error_buf, error_buf_size), in which args is a structure containing stack_size, heap_size, max_page_count and so on, so as to specify the max memory size when instantiating.
Enrico Loparco (Mar 02 2024 at 22:04):Another method may be adding another API like wasm_module_inst_t wasm_runtime_instantiate_ex(module, args, error_buf, error_buf_size)
Yes, that approach seems better since it avoids side effects, i.e. max memory value different from the one defined in the module being used without the developer knowing.
Last updated: Oct 23 2024 at 20:03 UTC