AlexEne opened Issue #1902:
<!-- Please try to describe precisely what you would like to do in
Cranelift/Wasmtime and/or expect from it. You can answer the questions below if
they're relevant and delete this text before submitting. Thanks for opening an
issue! -->Feature
<!-- What is the feature or code improvement you would like to do in
Cranelift/Wasmtime? -->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 callingwasi_instance_new
?Benefit
<!-- What is the value of adding this in Cranelift/Wasmtime? -->
Would clarify the ownership for the C/C++ usage of the wasmtime API.Implementation
<!-- Do you have an implementation plan, and/or ideas for data structures or
algorithms to use? -->
I can probably amend the example if needed.Alternatives
<!-- Have you considered alternative implementations? If so, how are they
better or worse than your proposal? -->
None
AlexEne edited Issue #1902:
<!-- Please try to describe precisely what you would like to do in
Cranelift/Wasmtime and/or expect from it. You can answer the questions below if
they're relevant and delete this text before submitting. Thanks for opening an
issue! -->Feature
<!-- What is the feature or code improvement you would like to do in
Cranelift/Wasmtime? -->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
andwasm_module_instance
that was linked to the wasi_instance?Is it safe to call
wasi_config_delete(...)
after callingwasi_instance_new
?Benefit
<!-- What is the value of adding this in Cranelift/Wasmtime? -->
Would clarify the ownership for the C/C++ usage of the wasmtime API.Implementation
<!-- Do you have an implementation plan, and/or ideas for data structures or
algorithms to use? -->
I can probably amend the example if needed.Alternatives
<!-- Have you considered alternative implementations? If so, how are they
better or worse than your proposal? -->
None
AlexEne commented on Issue #1902:
Also the linker probably needs to be deleted at some point too.
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;
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 awasmtime_instance_new
call:
wasm_instance_t *instance = NULL;
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)
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 callswasmtime_instance_new
. It looks like in the wasi example there's a missingwasmtime_linker_instantiate
.Is this example the correct way correct way of using the linker + wasi API?
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 callswasmtime_instance_new
. It looks like in the wasi example there's a missingwasmtime_linker_instantiate
.Is this example the correct way correct way of using the linker + wasi API?
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 callswasmtime_instance_new
. It looks like in the wasi example there's a missingwasmtime_linker_instantiate
.Is this example the correct way correct way of using the linker + wasi lib?
alexcrichton commented on Issue #1902:
Oh sorry so to clarify, the
own
annotation inwasm.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 ofwasm_engine_new_with_config
that's actually one of the few APIs that takes ownership of theconfig
. 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 sincewasmtime_linker_get_default
is used to fetch the function to call. For WASIwasmtime_linker_module
is implicitly instantiating the provided module.
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.
AlexEne closed Issue #1902:
<!-- Please try to describe precisely what you would like to do in
Cranelift/Wasmtime and/or expect from it. You can answer the questions below if
they're relevant and delete this text before submitting. Thanks for opening an
issue! -->Feature
<!-- What is the feature or code improvement you would like to do in
Cranelift/Wasmtime? -->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
andwasm_module_instance
that was linked to the wasi_instance?Is it safe to call
wasi_config_delete(...)
after callingwasi_instance_new
?Benefit
<!-- What is the value of adding this in Cranelift/Wasmtime? -->
Would clarify the ownership for the C/C++ usage of the wasmtime API.Implementation
<!-- Do you have an implementation plan, and/or ideas for data structures or
algorithms to use? -->
I can probably amend the example if needed.Alternatives
<!-- Have you considered alternative implementations? If so, how are they
better or worse than your proposal? -->
None
Last updated: Dec 23 2024 at 12:05 UTC