-
Notifications
You must be signed in to change notification settings - Fork 43
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 race conditions with functions returning "no output" #102
Conversation
Since the commit getditto@b191ff4 is based off the crates.io release (to reduce the churn from 1 The fix can be tested using…[patch.crates-io.node-bindgen]
version = "3.0.0"
git = "https://github.com/getditto/node-bindgen"
rev = "b191ff4d671418ee56cc99b75acfd1853cdab6e6" |
The current implementation of `#[node_bindgen]` wrapping shim over `async fn` functions special cased those with no return value to return immediately, for some reason. This meant that if such an `async fn` had a side-effect that following-up code wanted to observe, there would be a race condition whereby such code could observe causally-inconsitent state, leading to logic bugs.
b191ff4
to
038c012
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for contribution.
After removing cleanup, can you also bump up crate version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great.
This commit adds the test case component to infinyon#102 to ensure that this behavior is not regressed on.
This commit adds the test case component to #102 to ensure that this behavior is not regressed on.
The current implementation of
#[node_bindgen]
wrapping shim overasync fn
functions special cased those with no return value to return immediately. This meant that if such anasync fn
had a side-effect that following-up code wanted to observe, there would be a race condition whereby such code could observe causally-inconsistent state, leading to logic bugs.Removing that special-cased branch seems to have fixed the issue.