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

WASI target: thread::sleep() is panicing: unexpected result of poll_oneoff #65607

Closed
dunnock opened this issue Oct 19, 2019 · 3 comments · Fixed by #65617
Closed

WASI target: thread::sleep() is panicing: unexpected result of poll_oneoff #65607

dunnock opened this issue Oct 19, 2019 · 3 comments · Fixed by #65617
Labels
C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@dunnock
Copy link

dunnock commented Oct 19, 2019

Following code seems correct though will always panic when compiled to wasm32-wasi:

fn main() { std::thread::sleep(std::time::Duration::new(0,5)); }

compiled with nightly rust 1.40:
>cargo +nightly build --target wasm32-wasi

It fails at least in wasmtime and wasmer-js VMs (example https://github.com/dunnock/wabench/tree/master/poll_oneoff_test):

> wasmtime target/wasm32-wasi/debug/poll_oneoff_test.wasm

thread 'main' panicked at 'thread::sleep(): unexpected result of poll_oneoff', src/libstd/sys/wasi/thread.rs:61:18
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace.
error: failed to process main module target/wasm32-wasi/debug/poll_oneoff_test.wasm
    caused by: Instantiation error: Trap occurred while invoking start function: wasm trap: unreachable, source location: @8f01

rustup show:

active toolchain
nightly-x86_64-apple-darwin (default)
rustc 1.40.0-nightly (0e8a4b441 2019-10-16)
@dunnock
Copy link
Author

dunnock commented Oct 19, 2019

After some more digging it seems down to std::thread::sleep implementation

In __wasi_subscription_t.userdata passed 0 to poll_oneoff, which should set resulting event structure with same number __wasi_event_t.userdata:
https://github.com/rust-lang/rust/blob/master/src/libstd/sys/wasi/thread.rs#L45

Later __wasi_event_t.userdata matched to CLOCK_ID == 0x0123_45678:
https://github.com/rust-lang/rust/blob/master/src/libstd/sys/wasi/thread.rs#L56

Following code is a copy from std::thread::sleep's wasi implementation:

fn debug_std_thread_sleep() {
    // Copied from thread::sleep
    const CLOCK_ID: wasi::Userdata = 0x0123_45678;

    let clock = wasi::raw::__wasi_subscription_u_clock_t {
        identifier: CLOCK_ID,
        clock_id: wasi::CLOCK_MONOTONIC,
        timeout: 5u64,
        precision: 0,
        flags: 0,
    };
    let in_ = [wasi::Subscription {
        userdata: 0, //Should it be CLOCK_ID?
        type_: wasi::EVENTTYPE_CLOCK,
        u: wasi::raw::__wasi_subscription_u { clock: clock },
    }];
    let (res, event) = unsafe {
        let mut out: [wasi::Event; 1] = std::mem::zeroed();
        let res = wasi::poll_oneoff(&in_, &mut out);
        (res, out[0])
    };

    println!("Res = {:?}", res);
    assert_eq!(event.userdata, CLOCK_ID, "event.userdata");
    assert_eq!(event.error, 0, "event.error");
    assert_eq!(event.type_, wasi::EVENTTYPE_CLOCK, "event.type_");
}

Will show:

Res = Ok(1)
thread 'main' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `305419896`: event.userdata', src/main.rs:32:5

@mati865
Copy link
Contributor

mati865 commented Oct 19, 2019

cc @newpavlov

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 19, 2019
@newpavlov
Copy link
Contributor

Yes, it looks like CLOCK_ID should indeed go to the userdata field instead of identifier. I was not 100% sure about correctness of the implementation myself and it seems I've misinterpreted the Core API docs at the time (both fields have type __wasi_userdata_t).

JohnTitor added a commit to JohnTitor/rust that referenced this issue Oct 22, 2019
Fix WASI sleep impl

Closes rust-lang#65607

@sunfishcode
Is it fine to use 0 for the `identifier` field? What is this field used for?
@bors bors closed this as completed in ff2442f Oct 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-wasm Target: WASM (WebAssembly), http://webassembly.org/ T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants