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

Support WASI poll_oneoff for clock events #625

Closed
clarkmcc opened this issue Jun 8, 2022 · 14 comments · Fixed by #629
Closed

Support WASI poll_oneoff for clock events #625

clarkmcc opened this issue Jun 8, 2022 · 14 comments · Fixed by #629
Assignees
Labels
enhancement New feature or request

Comments

@clarkmcc
Copy link

clarkmcc commented Jun 8, 2022

Is your feature request related to a problem? Please describe.
Related to #271. The following code in Rust causes the following panic.

std::thread::sleep(time::Duration::from_secs(10));
thread '<unnamed>' panicked at 'thread::sleep(): unexpected result of poll_oneoff', library/std/src/sys/wasi/thread.rs:57:22

Describe the solution you'd like
Support for the poll_oneoff WASI method.

@clarkmcc clarkmcc added the enhancement New feature or request label Jun 8, 2022
@codefromthecrypt
Copy link
Contributor

excellent. I'll help with this unless you are game.

@codefromthecrypt codefromthecrypt self-assigned this Jun 15, 2022
@codefromthecrypt
Copy link
Contributor

It appears this is also used in zig, though not sure the code and compilation instructions that generates the binding https://github.com/ziglang/zig/blob/0e6285c8fc31ff866df96847fe34e660da38b4a9/lib/std/time.zig#L24-L47

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Jun 15, 2022

@mathetake
Copy link
Member

lol I completely forgot about that :))

@codefromthecrypt
Copy link
Contributor

@clarkmcc as there's more than one way to do it, if you have a moment, can you make a scratch repo to reproduce that error? I used wasm32-wasi locally and it failed but invisibly, as opposed to the error you found. I want to make sure I'm doing the same so I can sort it. Main thing is I want to iterate on this, and firstly solve the thread.sleep thing which is reasonably common across compilers.

@clarkmcc
Copy link
Author

@codefromthecrypt here's a reproduction https://github.com/clarkmcc/wazero-issue-625. I believe the key is to pipe stdout and stderr so that the Rust panic gets passed back to Go.

https://github.com/clarkmcc/wazero-issue-625/blob/f36171ac363dafa997109f179a3226c0339caf1f/main.go#L28-L29

@codefromthecrypt
Copy link
Contributor

thanks, @clarkmcc!

self-note: here's the wasi-libc call site for sleep, though there are other uses of poll_oneoff https://github.com/WebAssembly/wasi-libc/blob/659ff414560721b1660a19685110e484a081c3d4/libc-bottom-half/cloudlibc/src/libc/time/clock_nanosleep.c

@codefromthecrypt
Copy link
Contributor

I did some research and the best way to incrementally implement this is to cheat similarly to how wasmtime and wasi-libc do, where they hook into sleep when the poll event is a clock. ex bytecodealliance/wasmtime#2753

@codefromthecrypt codefromthecrypt changed the title Support WASI poll_oneoff Support WASI poll_oneoff for clock events Jun 16, 2022
@codefromthecrypt
Copy link
Contributor

by cheat I mean use one sleeper for both supported clocks (wall and nano) due to an interpretation of "clock_settime" not affecting relative time https://linux.die.net/man/3/clock_settime

@codefromthecrypt
Copy link
Contributor

#629 works, but I need to backfill tests and check other impls

@codefromthecrypt
Copy link
Contributor

Notes about go-generated wasm, doesn't use wasi, rather implements sleep with nanotime.


GOOS=js GOARCH=wasm compiled wasm uses nanotime as that's what time.Sleep does internally (among atomic ops that aren't really atomic in go-generated wasm).

Ex. the following only uses nanotime when compiled with GOOS=js GOARCH=wasm

package main

import "time"

func main() {
	time.Sleep(time.Second * 10)
}

related, GOOS=js GOARCH=wasm uses notetsleepg which calls both nanotime and scheduleTimeoutEvent. I'm putting timeout events out of scope even though there's some future need in wasi-clocks

cc @prep

@codefromthecrypt
Copy link
Contributor

verified tinygo uses what we think it does

@codefromthecrypt
Copy link
Contributor

verified zig thanks to @mathetake for the help gettin ziggy witit

$ zig build-exe -O ReleaseSmall -target wasm32-wasi sleep.zig
const std = @import("std");
pub fn main() !void {
    std.time.sleep(std.time.ns_per_s * 5);
}

@codefromthecrypt
Copy link
Contributor

thanks for the patience. #629 should sort this out and also do so incrementally because implementing all of poll_oneoff would be quite a task both for the coder and reviewer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants