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

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

Open
rvolosatovs opened this issue Jan 18, 2024 · 1 comment
Open
Labels
bug Incorrect behavior in the current implementation that needs fixing

Comments

@rvolosatovs
Copy link
Member

rvolosatovs commented Jan 18, 2024

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

let scope = self.calls.scopes.len() - 1;

Versions and Environment

Wasmtime version or commit: f3b5478

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

BORROW => (tables.resource_lower_borrow(None, rep), None),
, note that in case of Resource, the analogous operation simply returns the rep

Refs #7688 #7783

@rvolosatovs rvolosatovs added the bug Incorrect behavior in the current implementation that needs fixing label Jan 18, 2024
@alexcrichton
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Incorrect behavior in the current implementation that needs fixing
Projects
None yet
Development

No branches or pull requests

2 participants