Skip to content

Commit

Permalink
Merge #2084
Browse files Browse the repository at this point in the history
2084: fix(c-api) Avoid more than one call to the function environment finalizer r=Hywan a=Hywan

# Description

A function environment `WrapperEnv` can be cloned. In case the
original and the cloned environments are being dropped, the
`self.finalizer` function —if any— is called twice. That's a bug. It
must be called only once.

This patch updates the code to apply the finalizer only once by taking
it —if any— the first time it's called.

This bug has been raised by @jubianchi when working with PHP and the Zend Engine GC.

# Review

- [x] Add a short description of the the change to the CHANGELOG.md file


Co-authored-by: Ivan Enderlin <[email protected]>
  • Loading branch information
bors[bot] and Hywan authored Feb 1, 2021
2 parents 1cea9cb + b954429 commit 05f356f
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [#2056](https://github.com/wasmerio/wasmer/pull/2056) Change back to depend on the `enumset` crate instead of `wasmer_enumset`

### Fixed
- [#2084](https://github.com/wasmerio/wasmer/pull/2084) Avoid calling the function environment finalizer more than once when the environment has been cloned in the C API.
- [#2069](https://github.com/wasmerio/wasmer/pull/2069) Use the new documentation for `include/README.md` in the Wasmer package.
- [#2042](https://github.com/wasmerio/wasmer/pull/2042) Parse more exotic environment variables in `wasmer run`.
- [#2041](https://github.com/wasmerio/wasmer/pull/2041) Documentation diagrams now have a solid white background rather than a transparent background.
Expand Down
16 changes: 12 additions & 4 deletions lib/c-api/src/wasm_c_api/externals/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ pub unsafe extern "C" fn wasm_func_new_with_env(
#[repr(C)]
struct WrapperEnv {
env: *mut c_void,
finalizer: Option<wasm_env_finalizer_t>,
finalizer: Arc<Option<wasm_env_finalizer_t>>,
};

// Only relevant when using multiple threads in the C API;
Expand All @@ -115,8 +115,13 @@ pub unsafe extern "C" fn wasm_func_new_with_env(

impl Drop for WrapperEnv {
fn drop(&mut self) {
if let Some(finalizer) = self.finalizer {
unsafe { (finalizer)(self.env as _) }
if let Some(finalizer) = Arc::get_mut(&mut self.finalizer)
.map(Option::take)
.flatten()
{
if !self.env.is_null() {
unsafe { (finalizer)(self.env as _) }
}
}
}
}
Expand Down Expand Up @@ -160,7 +165,10 @@ pub unsafe extern "C" fn wasm_func_new_with_env(
let function = Function::new_with_env(
&store.inner,
func_sig,
WrapperEnv { env, finalizer },
WrapperEnv {
env,
finalizer: Arc::new(finalizer),
},
inner_callback,
);

Expand Down

0 comments on commit 05f356f

Please sign in to comment.