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

Panic when reentering wasm_func_call from C API. #365

Closed
peterhuene opened this issue Sep 24, 2019 · 0 comments · Fixed by #366
Closed

Panic when reentering wasm_func_call from C API. #365

peterhuene opened this issue Sep 24, 2019 · 0 comments · Fixed by #366

Comments

@peterhuene
Copy link
Member

peterhuene commented Sep 24, 2019

Repro

With the following WASM module:

(module
  (type $t0 (func (result i32 i32)))
  (import "" "get" (func $.get (type $t0)))
  (func $add (result i32)
    call $.get
    i32.add
  )
  (export "add" (func $add))
  (func $a (result i32)
    i32.const 1
  )
  (export "a" (func $a))
  (func $b (result i32)
    i32.const 2
  )
  (export "b" (func $b))
)

Define an import for get that calls exported functions a and b on the instance (here is an example of doing so from a prototype C# binding for Wasmtime):

class Host : Wasmtime.Host
{
    [Import("get")]
    public (int, int) Get()
    {
        var instance = (dynamic)Instance;
        return (instance.a(), instance.b());
    }
}

class Program
{
    static void Main(string[] args)
    {
        using (var engine = new Engine())
        {
            using (var store = engine.CreateStore())
            {
                using (var module = store.CreateModule("test.wasm"))
                {
                    using (dynamic instance = module.Instantiate<Host>())
                    {
                        Console.WriteLine($"Function returned: {instance.add()}.");
                    }
                }
            }
        }
    }
}

Expected Result

The program should output:

Function returned: 3.

Actual Result

The program panics:

thread '<unnamed>' panicked at 'already borrowed: BorrowMutError', src/libcore/result.rs:1084:5
stack backtrace:
   0: std::panicking::default_hook::{{closure}}
   1: std::panicking::default_hook
   2: std::panicking::rust_panic_with_hook
   3: std::panicking::continue_panic_fmt
   4: rust_begin_unwind
   5: core::panicking::panic_fmt
   6: core::result::unwrap_failed
   7: core::result::Result<T,E>::expect
   8: core::cell::RefCell<T>::borrow_mut
   9: <wasmtime_api::callable::WasmtimeFn as wasmtime_api::callable::WrappedCallable>::call
  10: wasmtime_api::externals::Func::call
  11: wasm_func_call

Additional Information

The problem appears to be the scope of the borrow for store, which remains borrowed across the call to wasmtime_call_trampoline. This prevents any imported function from calling an exported function.

@peterhuene peterhuene changed the title Panic when reentering wasm_func_call. Panic when reentering wasm_func_call from C API. Sep 24, 2019
peterhuene added a commit to peterhuene/wasmtime that referenced this issue Sep 24, 2019
This commit fixes the borrow scope of `store` in `wasm_func_call` such that it
does not remain borrowed across the call to `wasmtime_call_trampoline`.  By
limiting the scope of the borrow, `wasm_func_call` can be reentered if an
exported function calls an imported function, which in turn calls another
exported function.

Fixes bytecodealliance#365.
peterhuene added a commit to peterhuene/wasmtime that referenced this issue Sep 25, 2019
This PR fixes the borrow scope of store in the `WrappedCallable` impl of
`WasmTimeFn` such that it does not remain borrowed across the call to
`wasmtime_call_trampoline`. By limiting the scope of the borrow, the
implementation can be reentered if an exported function calls an imported
function, which in turn calls another exported function.

Fixes bytecodealliance#365.
peterhuene added a commit to peterhuene/wasmtime that referenced this issue Sep 26, 2019
This PR fixes the borrow scope of store in the `WrappedCallable` impl of
`WasmTimeFn` such that it does not remain borrowed across the call to
`wasmtime_call_trampoline`. By limiting the scope of the borrow, the
implementation can be reentered if an exported function calls an imported
function, which in turn calls another exported function.

Fixes bytecodealliance#365.
sunfishcode pushed a commit that referenced this issue Sep 26, 2019
This PR fixes the borrow scope of store in the `WrappedCallable` impl of
`WasmTimeFn` such that it does not remain borrowed across the call to
`wasmtime_call_trampoline`. By limiting the scope of the borrow, the
implementation can be reentered if an exported function calls an imported
function, which in turn calls another exported function.

Fixes #365.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant