Stream: git-wasmtime

Topic: wasmtime / issue #6359 add C API for epoch_deadline_callb...

Wasmtime GitHub notifications bot (May 09 2023 at 14:44):

github-actions[bot] commented on issue #6359:

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 <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (May 10 2023 at 18:52):

kpreisser commented on issue #6359:

Hi! From my testing, it seems the new wasmtime_error_new function in the C API doesn't copy the string passed by the const char* argument when being called; instead the supplied char pointer is dereferenced later after wasmtime_error_new has already returned, e.g. when returning the wasmtime_error_t* in the epoch callback. Is this intended?

I would have expected a similar behavior as wasmtime_trap_new, which copies the string when being called, so that the memory holding the string can be freed by the caller after the function returns. Otherwise, it's not quite clear to me when the memory can be freed.

Thanks!

Wasmtime GitHub notifications bot (May 10 2023 at 18:57):

kpreisser edited a comment on issue #6359:

Hi! From my testing, it seems the new wasmtime_error_new function in the C API doesn't copy the string passed by the const char* argument when being called; instead the supplied char pointer is dereferenced later after wasmtime_error_new has already returned, e.g. when returning the wasmtime_error_t* in the epoch deadline callback. Is this intended?

I would have expected a similar behavior as wasmtime_trap_new, which copies the string when being called, so that the memory holding the string can be freed by the caller after the function returns. Otherwise, it's not quite clear to me when the memory can be freed.

Thanks!

Wasmtime GitHub notifications bot (May 10 2023 at 18:57):

alexcrichton commented on issue #6359:

Ah thanks for testing! That's a mistake in the API and a subtle detail with the implementation. It's definitely not intended that the pointer should remain live, the contents should be copied into the error.

The line that needs to change is this one, namely the anyhow!(msg_string) should become anyhow!("{msg_string}"). @theothergraham or @kpreisser would one of y'all be up for a PR along those lines?

Wasmtime GitHub notifications bot (May 10 2023 at 19:38):

theothergraham commented on issue #6359:

:face_palm: Sorry, that's my Rust newbieness showing. I missed that String:: from_utf8_lossy() only makes a copy if it finds invalid uft-8. I'll make a PR. Thank you for catching that, @kpreisser!

Last updated: Oct 23 2024 at 20:03 UTC