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(sdk-common, wasm): implment JoinHandle correctly #3331

Closed
wants to merge 2 commits into from

Conversation

maan2003
Copy link
Contributor

@maan2003 maan2003 commented Apr 15, 2024

fixes task getting cancelled on drop of join handle

RemoteHandle already aborts on drop, we don't need abort handle

Signed-off-by: Manmeet Singh <[email protected]>
@maan2003 maan2003 requested a review from a team as a code owner April 15, 2024 16:56
@maan2003 maan2003 requested review from bnjbvr and removed request for a team April 15, 2024 16:56
@maan2003
Copy link
Contributor Author

looks like I broke some stuff, my work based on old commit. will fix

@maan2003 maan2003 marked this pull request as draft April 15, 2024 17:00
@maan2003 maan2003 force-pushed the push-kqoknowkxsno branch 2 times, most recently from e635fec to 52fc232 Compare April 15, 2024 17:09
Comment on lines -62 to +51
pub fn abort(&self) {
self.abort_handle.abort();
pub fn abort(&mut self) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note: it requires &mut, but I think it is fine.

@maan2003 maan2003 changed the title sdk-common: implment JoinHandle correctly fix(sdk-common, wasm): implment JoinHandle correctly Apr 15, 2024
@maan2003 maan2003 marked this pull request as ready for review April 15, 2024 18:53
Signed-off-by: Manmeet Singh <[email protected]>
@maan2003
Copy link
Contributor Author

closing in favor of #3333

@maan2003 maan2003 closed this Apr 15, 2024
@maan2003 maan2003 deleted the push-kqoknowkxsno branch April 16, 2024 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant