bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.21k stars 1.28k forks source link

`Resource::new_borrow()` resource -> `ResourceAny` conversion panics #7793

Open rvolosatovs opened 8 months ago

rvolosatovs commented 8 months ago

Test Case

#[test]
fn can_use_own_for_borrow() -> Result<()> {
    let engine = super::engine();
    let c = Component::new(
        &engine,
        r#"
            (component
                (import "t" (type $t (sub resource)))

                (core func $drop (canon resource.drop $t))

                (core module $m
                    (import "" "drop" (func $drop (param i32)))
                    (func (export "f") (param i32)
                        (call $drop (local.get 0))
                    )
                )
                (core instance $i (instantiate $m
                    (with "" (instance
                        (export "drop" (func $drop))
                    ))
                ))

                (func (export "f") (param "x" (borrow $t))
                    (canon lift (core func $i "f")))
            )
        "#,
    )?;

    struct MyType;

    let mut store = Store::new(&engine, ());
    let mut linker = Linker::new(&engine);
    let ty_idx = linker
        .root()
        .resource("t", ResourceType::host::<MyType>(), |_, _| Ok(()))?;
    let i_pre = linker.instantiate_pre(&c)?;
    Resource::<MyType>::new_borrow(100).try_into_resource_any(&mut store, &i_pre, ty_idx)?;
}

Steps to Reproduce

(see test case)

Expected Results

Success, just like with lowering of Resource directly

Actual Results

Panic in https://github.com/bytecodealliance/wasmtime/blob/f3b5478bfcb759d99b3910b121c644b4c9c572bf/crates/runtime/src/component/resources.rs#L241

Versions and Environment

Wasmtime version or commit: https://github.com/bytecodealliance/wasmtime/commit/f3b5478bfcb759d99b3910b121c644b4c9c572bf

Extra Info

The panic makes sense, since there is no "owned" version of this resource in any table, only a "synthetic" borrow.

I believe the panic is caused by https://github.com/bytecodealliance/wasmtime/blob/f3b5478bfcb759d99b3910b121c644b4c9c572bf/crates/wasmtime/src/component/resources.rs#L568, note that in case of Resource, the analogous operation simply returns the rep https://github.com/bytecodealliance/wasmtime/blob/f3b5478bfcb759d99b3910b121c644b4c9c572bf/crates/wasmtime/src/component/resources.rs#L334

Refs #7688 #7783

alexcrichton commented 8 months ago

Ah yes I see where this is coming from and it's a bit unfortunate. Borrows have metadata tracking them to ensure that they're all dropped by the time a function call exits. For example if 4 borrows are given to an exported function then all borrows must be dropped by the exported function before the function returns, otherwise the component model requires a trap.

I think that the implementation here is going to have to be a bit more "clever" like the bits in Resource<T> where the lowering into a borrow is actually deferred until the lift/lower operation happens. Basically resource_lower_borrow can't be called until a function is being invoked.