clone wasm store engine (#3542)

This resolves https://github.com/tree-sitter/tree-sitter/issues/3454.

This brings the usage of wasmtime::Engine in line with how wasmtime
intends it to be used. All wasmtime functions that receive an Engine
always receive an `&Engine`, never an owned `Engine`.  They are always
responsible for cloning the reference if they need it.

This brings the usage of wasmtime::Engine in line with how TSParser
treats TSLanguages: when setting a language to the parser, the parser is
responsible for cloning the reference to the TSLanguage. It is
counterintuitive for TSParser to have different behavior when receiving
wasmtime_engine_t.

C API users also expect this behavior, see "Memory Management"
[here](https://docs.wasmtime.dev/c-api/wasm_8h.html). Talking about the
C API: without this change, failing to clone the `wasmtime_engine_t`
(which, again, is never something API users need to do in wasmtime) and
then reusing the engine in multiple TSLanguages results in a use after
free. With this change, failing to call `wasm_engine_delete` on your
owned Engine results in a memory leak. Memory leaks are safer than
use-after-free.
This commit is contained in:
Ryan Patterson 2024-08-22 21:01:37 +06:00 committed by GitHub
parent 5364ac4ea8
commit b5e4ef6d9a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 20 additions and 21 deletions

View file

@ -41,12 +41,11 @@ pub enum WasmErrorKind {
}
impl WasmStore {
pub fn new(engine: wasmtime::Engine) -> Result<Self, WasmError> {
pub fn new(engine: &wasmtime::Engine) -> Result<Self, WasmError> {
unsafe {
let mut error = MaybeUninit::<ffi::TSWasmError>::uninit();
let engine = Box::new(wasm_engine_t { engine });
let store = ffi::ts_wasm_store_new(
(Box::leak(engine) as *mut wasm_engine_t).cast(),
(engine as *const wasmtime::Engine as *mut wasmtime::Engine).cast(),
error.as_mut_ptr(),
);
if store.is_null() {