C API: expose wasmtime_linker_get_one_by_name()

[thibaultcha]

I'd like to use the new Linker to manage my modules from C, is this a supported use-case for the Linker?

[github-actions]

Subscribe to Label Action

cc @peterhuene

Details

This issue or pull request has been labeled: "wasmtime:c-api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:c-api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

Thanks for this! To confirm, have you tried out using this API in your application? We don't have a ton of tests for the C API right now and we mostly rely on external testing for it.

[thibaultcha]

Hi there!

Yes, I have been using this in my embedding and its been working fine, and returning the Extern as expected.

However, there has been something bugging me, and I am not sure if it is related or not. I am observing wasmtime_func_call() leaking some memory every time it is invoked. Details:

I've been using this new API like so:

// static_memory_maximum_size = 0 for testing

wasm_name_t        mod_name,  func_name;
wasm_extern_t     *func_extern;
wasm_func_t       *func;

/* ... */

error = wasmtime_linker_get_one_by_name(linker, &mod_name, &func_name, &func_extern);
if (error) {
    /* ... */
}

func = wasm_extern_as_func(func_extern); // this function is a nop

error = wasmtime_func_call(func, NULL, 0, NULL, 0, &trap);

wasm_extern_delete(func_extern);

if (error || trap) {
    /* ... */
}

In a gdb session, I observe 1152Kb being allocated here and never freed. This amount of memory sounded a lot like the default wasm stack to me, but even when I update the stack size in my Engine's config, I still see the same 1152Kb allocations. When I remove the static_memory_maximum_size = 0 setting, I see the 6Gb of static memory being allocated and never freed for every call. Aren't these Externs in the Linker already backed by an underlying Instance?

Am I missing something here? I'm not sure if this is caused by a mistake in my use of wasmtime_func_call() or in this new C API...

[alexcrichton]

Oh dear that does sound bad!

Could this perhaps be the instance-per-function-call logic of Linker? Is the function that you're calling attached to a wasi command which means that we'll spin up one instance per function call, and then the instances aren't destroyed until the Store is dropped?

[thibaultcha]

It does make it hard to use indeed! I found out what the issue is, although I cannot explain quite why without digging into the Linker. Said Linker does not have WASI defined, and the function was declared in a module as such:

#[no_mangle]
pub fn _start() {
    // nop
}

Turns out that changing the name from _start to, e.g. my_func does not trigger the allocation/leak issue anymore. I recall that _start isn't really supposed to be part of a Reactor module (am I wrong?), so I don't mind much renaming it; nonetheless, the behaviour seems to highlight an underlying, problematic issue, doesn’t it?

[thibaultcha]

One more somewhat related question on memory management via the C API: should I expect wasmtime_linker_delete()/wasm_store_delete()/wasm_engine_delete() to properly free all of the underlying instances of my Modules?

Valgrind seems to have a lot of complaints about me calling these, below is an excerpt of the Valgrind output when calling the above 3 functions on my single Linker/Store/Engine during my application's exit:

==3218139== 488 bytes in 1 blocks are still reachable in loss record 66 of 97
==3218139==    at 0x483A809: malloc (vg_replace_malloc.c:307)
==3218139==    by 0x5048BBB: alloc::alloc::alloc (alloc.rs:80)
==3218139==    by 0x5048FF3: <alloc::alloc::Global as core::alloc::AllocRef>::alloc (alloc.rs:174)
==3218139==    by 0x5048B14: alloc::alloc::exchange_malloc (alloc.rs:268)
==3218139==    by 0x507420D: alloc::sync::Arc<T>::new (sync.rs:323)
==3218139==    by 0x5055E7A: wasmtime_jit::instantiate::CompiledModule::new (instantiate.rs:145)
==3218139==    by 0x4FB2663: wasmtime::module::Module::compile (module.rs:304)
==3218139==    by 0x4FB249F: wasmtime::module::Module::from_binary_unchecked (module.rs:275)
==3218139==    by 0x4FB2469: wasmtime::module::Module::from_binary (module.rs:244)
==3218139==    by 0x4B1BEFD: wasmtime_module_new (module.rs:56)
==3218139==    by 0x4DA9B1: [my_embedding] ([my_embedding].c:298)
==3218139==
==3218139== 640 bytes in 8 blocks are still reachable in loss record 72 of 97
==3218139==    at 0x483A809: malloc (vg_replace_malloc.c:307)
==3218139==    by 0x5EDC62F: alloc (alloc.rs:80)
==3218139==    by 0x5EDC62F: alloc (alloc.rs:174)
==3218139==    by 0x5EDC62F: exchange_malloc (alloc.rs:268)
==3218139==    by 0x5EDC62F: new<std::thread::Inner> (sync.rs:323)
==3218139==    by 0x5EDC62F: std::thread::Thread::new (mod.rs:1141)
==3218139==    by 0x598D95D: std::thread::Builder::spawn_unchecked (mod.rs:462)
==3218139==    by 0x598E2CB: std::thread::Builder::spawn (mod.rs:386)
==3218139==    by 0x596FB58: <rayon_core::registry::DefaultSpawn as rayon_core::registry::ThreadSpawn>::spawn (registry.rs:101)
==3218139==    by 0x5970862: rayon_core::registry::Registry::new (registry.rs:257)
==3218139==    by 0x596FDCD: rayon_core::registry::global_registry::{{closure}} (registry.rs:168)
==3218139==    by 0x596FF59: rayon_core::registry::set_global_registry::{{closure}} (registry.rs:194)
==3218139==    by 0x5973C23: std::sync::once::Once::call_once::{{closure}} (once.rs:264)
==3218139==    by 0x5EE6F47: std::sync::once::Once::call_inner (once.rs:416)
==3218139==    by 0x5973B50: std::sync::once::Once::call_once (once.rs:264)
==3218139==    by 0x596FE9A: rayon_core::registry::set_global_registry (registry.rs:193)
==3218139== 
==3218139== 704 bytes in 1 blocks are indirectly lost in loss record 73 of 97
==3218139==    at 0x483A809: malloc (vg_replace_malloc.c:307)
==3218139==    by 0x4B3BA4B: alloc::alloc::alloc (alloc.rs:80)
==3218139==    by 0x4B3BE83: <alloc::alloc::Global as core::alloc::AllocRef>::alloc (alloc.rs:174)
==3218139==    by 0x4AE9B76: alloc::raw_vec::RawVec<T,A>::allocate_in (raw_vec.rs:152)
==3218139==    by 0x4AEDBDB: alloc::raw_vec::RawVec<T,A>::with_capacity_in (raw_vec.rs:135)
==3218139==    by 0x4AE62FD: alloc::raw_vec::RawVec<T>::with_capacity (raw_vec.rs:92)
==3218139==    by 0x4B4075D: alloc::vec::Vec<T>::with_capacity (vec.rs:358)
==3218139==    by 0x4B4DC63: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter (vec.rs:2073)
==3218139==    by 0x4B5146B: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter (vec.rs:1981)
==3218139==    by 0x4AFF6EF: core::iter::traits::iterator::Iterator::collect (iterator.rs:1660)
==3218139==    by 0x4B3AEB8: wasmtime::module::wasmtime_module_new::{{closure}} (module.rs:61)
==3218139==    by 0x4B2858D: wasmtime::error::handle_result (error.rs:30)

...

[alexcrichton]

I've opened #1913 to track the leak issues you were encountering.

Otherwise for API usage you'll want to *_delete(..) every handle you're given ownership of, but if you do that then no memory should be leaked.

And finally, this looks good otherwise, so I'm going to go ahead and merge!

Feel free to ping me on Zulip if you've still got memory issues though

[thibaultcha]

@alexcrichton I was writing something here to clarify that I read your comment at #1902 (comment) which answered my above question. Anyway, thank you for detailing it once again!

Thanks for the merge too.