Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(c-api) Avoid more than one call to the function environment finalizer #2084

Merged
merged 1 commit into from
Feb 1, 2021

Conversation

Hywan
Copy link
Contributor

@Hywan Hywan commented Feb 1, 2021

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

  • Add a short description of the the change to the CHANGELOG.md file

@Hywan Hywan added bug Something isn't working 📦 lib-c-api About wasmer-c-api labels Feb 1, 2021
@Hywan Hywan self-assigned this Feb 1, 2021
Hywan added a commit to Hywan/wasmer that referenced this pull request Feb 1, 2021
@Hywan Hywan changed the title fix(c-api) Avoid more than one call to the function environment finalizer. fix(c-api) Avoid more than one call to the function environment finalizer Feb 1, 2021
@Hywan Hywan force-pushed the fix-c-api-wasmerenv-func-finalizer branch from 9bdcf81 to 1357740 Compare February 1, 2021 14:34
…izer.

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.
@Hywan Hywan force-pushed the fix-c-api-wasmerenv-func-finalizer branch from 1357740 to b954429 Compare February 1, 2021 14:42
@Hywan
Copy link
Contributor Author

Hywan commented Feb 1, 2021

bors r+

Copy link
Contributor

@jubianchi jubianchi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved & tested agains wasmer-php

@bors
Copy link
Contributor

bors bot commented Feb 1, 2021

@bors bors bot merged commit 05f356f into wasmerio:master Feb 1, 2021
jubianchi added a commit to wasmerio/wasmer-php that referenced this pull request Feb 1, 2021
jubianchi added a commit to wasmerio/wasmer-php that referenced this pull request Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 📦 lib-c-api About wasmer-c-api
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants