wasmtime / Issue #1902 wasi_instance and wasi_config life... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / Issue #1902 wasi_instance and wasi_config life...

Wasmtime GitHub notifications bot (Jun 19 2020 at 12:53):

AlexEne opened Issue #1902:

Feature



The example here https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c isn't clear on what the correct lifetime management should be for the wasi_instance and wasi_config.

When we link teh wasi_instance, dp we still need to delete it?

What is the correct order to delete the wasi_instance and wasm_module_instance that was linked to the wasi_instance?

Is it safe to call wasi_config_delete(...) after calling wasi_instance_new?

Benefit


Would clarify the ownership for the C/C++ usage of the wasmtime API.

Implementation


I can probably amend the example if needed.

Alternatives


None

Wasmtime GitHub notifications bot (Jun 19 2020 at 12:54):

AlexEne edited Issue #1902:

Feature



The example here https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c isn't clear on what the correct lifetime management should be for the wasi_instance and wasi_config.

When we link the wasi_instance, do we still need to delete it?

What is the correct order to delete the wasi_instance and wasm_module_instance that was linked to the wasi_instance?

Is it safe to call wasi_config_delete(...) after calling wasi_instance_new?

Benefit


Would clarify the ownership for the C/C++ usage of the wasmtime API.

Implementation


I can probably amend the example if needed.

Alternatives


None

Wasmtime GitHub notifications bot (Jun 19 2020 at 12:55):

AlexEne commented on Issue #1902:

Also the linker probably needs to be deleted at some point too.

Wasmtime GitHub notifications bot (Jun 19 2020 at 13:15):

AlexEne edited a comment on Issue #1902:

Also the linker probably needs to be deleted at some point too.

This line needs to probably be deleted, tho I don't super understand how the module is instantiated, as we seem to miss a wasmtime_instance_new call.
wasm_instance_t *instance = NULL;

Wasmtime GitHub notifications bot (Jun 19 2020 at 13:22):

AlexEne edited a comment on Issue #1902:

Also the linker probably needs to be deleted at some point too.

Another thing I found in that example is that this line needs to probably be deleted.
I don't super understand how the module is instantiated, as we seem to miss a wasmtime_instance_new call:
wasm_instance_t *instance = NULL;

Wasmtime GitHub notifications bot (Jun 23 2020 at 18:12):

alexcrichton commented on Issue #1902:

Thanks for the report! It's a good point that the documentation on this is pretty lax!

Currently the memory management is that if you get an owned handle you must call delete on it to free its resources. Other than that you're free to delete your own personal handle for anything at any time. For example you can "delete" a wasm_store_t immediately after instantiation. Underlying resources won't get deallocated unless all references to the underlying resource go away. (aka everything is reference counted)

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:02):

AlexEne commented on Issue #1902:

This makes lots of sense. Thanks for explaining this it makes it easier for C++/C users as it was unclear what to do in the case of a failure. E.g:
wasm_config_t* config = wasm_config_new();
wasmtime_config_debug_info_set(config, true);

engine = wasm_engine_new_with_config(config);
Initially I assumed that methods 'take" ownership of data (e.g. wasi_engine_new_with_config would own the config from now on in the code above. I'm super glad it's not the case as it simplifies a lot for things for users of the API. Previously it wasn't clear what to do with config in case engine is returned null, was it de-allocated by the lib?, does it need to be de-allocated by the user of the lib? etc.

I think one good next step is to go through the examples for C/C++ and call free on all things that have come from a wasmtime_thing_new();. I can try raising a patch with that maybe in the following days if I get a chance.

On the secondary point in this particular example here: https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c, I am not sure how it works as it doesn't call anything that returns a module instance as far as I could tell. https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c#L83
Lower we see a function called from a module, but it conflicts with previous examples (without WASI and a linker), that first need to instantiate a module before using a function exported from it. E.g: https://github.com/bytecodealliance/wasmtime/blob/master/examples/hello.c#L90 calls wasmtime_instance_new. It looks like in the wasi example there's a missing wasmtime_linker_instantiate.

Is this example the correct way correct way of using the linker + wasi API?

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:02):

AlexEne edited a comment on Issue #1902:

This makes lots of sense. Thanks for explaining this it makes it easier for C++/C users as it was unclear what to do in the case of a failure. E.g:
wasm_config_t* config = wasm_config_new();
wasmtime_config_debug_info_set(config, true);

engine = wasm_engine_new_with_config(config);
Initially I assumed that methods 'take" ownership of data (e.g. wasi_engine_new_with_config would own the config from now on in the code above. I'm super glad it's not the case as it simplifies a lot for things for users of the API. Previously it wasn't clear what to do with config in case engine is returned null, was it de-allocated by the lib?, does it need to be de-allocated by the user of the lib? etc. Now all that is clear!

I think one good next step is to go through the examples for C/C++ and call free on all things that have come from a wasmtime_thing_new();. I can try raising a patch with that maybe in the following days if I get a chance.

On the secondary point in this particular example here: https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c, I am not sure how it works as it doesn't call anything that returns a module instance as far as I could tell. https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c#L83
Lower we see a function called from a module, but it conflicts with previous examples (without WASI and a linker), that first need to instantiate a module before using a function exported from it. E.g: https://github.com/bytecodealliance/wasmtime/blob/master/examples/hello.c#L90 calls wasmtime_instance_new. It looks like in the wasi example there's a missing wasmtime_linker_instantiate.

Is this example the correct way correct way of using the linker + wasi API?

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:15):

AlexEne edited a comment on Issue #1902:

This makes lots of sense. Thanks for explaining this it makes it easier for C++/C users as it was unclear what to do in the case of a failure. E.g:
wasm_config_t* config = wasm_config_new();
wasmtime_config_debug_info_set(config, true);

engine = wasm_engine_new_with_config(config);
Initially I assumed that methods 'take" ownership of data (e.g. wasi_engine_new_with_config would own the config from now on in the code above. I'm super glad it's not the case as it simplifies a lot for things for users of the API. Previously it wasn't clear what to do with config in case engine is returned null, was it de-allocated by the lib?, does it need to be de-allocated by the user of the lib? etc. Now all that is clear!

I think one good next step is to go through the examples for C/C++ and call free on all things that have come from a wasmtime_thing_new();. I can try raising a patch with that maybe in the following days if I get a chance.

On the secondary point in this particular example here: https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c, I am not sure how it works as it doesn't call anything that returns a module instance as far as I could tell. https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c#L83
Lower we see a function called from a module, but it conflicts with previous examples (without WASI and a linker), that first need to instantiate a module before using a function exported from it. E.g: https://github.com/bytecodealliance/wasmtime/blob/master/examples/hello.c#L90 calls wasmtime_instance_new. It looks like in the wasi example there's a missing wasmtime_linker_instantiate.

Is this example the correct way correct way of using the linker + wasi lib?

Wasmtime GitHub notifications bot (Jun 23 2020 at 21:36):

alexcrichton commented on Issue #1902:

Oh sorry so to clarify, the own annotation in wasm.h is the guide for when you own a piece of data. As a parameter it means you're given ownership to the function. In the case of wasm_engine_new_with_config that's actually one of the few APIs that takes ownership of the config. The function unconditionally takes ownership regardless of result value right now.

(and to be clear all of this could be better documented)

The examples should have all the appropriate *_delete calls and shouldn't leak memory, although something may have been missed!

For the WASI example I don't think that it's survived refactorings too well, it looks indeed like instance is unused since wasmtime_linker_get_default is used to fetch the function to call. For WASI wasmtime_linker_module is implicitly instantiating the provided module.

Wasmtime GitHub notifications bot (Jun 23 2020 at 22:32):

AlexEne commented on Issue #1902:

Alright, I think this solves all questions I had around it. Thanks for explaining it.
I will raise a small PR to the example delete the instance and add a few comments, but it doesn't need this issue to hang around so I can close it.

Wasmtime GitHub notifications bot (Jun 23 2020 at 22:32):

AlexEne closed Issue #1902:

Feature



The example here https://github.com/bytecodealliance/wasmtime/blob/master/examples/wasi/main.c isn't clear on what the correct lifetime management should be for the wasi_instance and wasi_config.

When we link the wasi_instance, do we still need to delete it?

What is the correct order to delete the wasi_instance and wasm_module_instance that was linked to the wasi_instance?

Is it safe to call wasi_config_delete(...) after calling wasi_instance_new?

Benefit


Would clarify the ownership for the C/C++ usage of the wasmtime API.

Implementation


I can probably amend the example if needed.

Alternatives


None

Last updated: Nov 22 2024 at 16:03 UTC