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

Add a Steal() method to Client and ClientSnapshot #522

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zenhack
Copy link
Contributor

@zenhack zenhack commented May 27, 2023

...and use it to catch a bug; the test is failing, need to track it down.

@zenhack zenhack marked this pull request as draft May 27, 2023 19:46
zenhack added 2 commits May 27, 2023 15:49
...and use it to catch a bug; the test is failing, need to track it
down.
@zenhack zenhack marked this pull request as ready for review May 27, 2023 20:26
@zenhack
Copy link
Contributor Author

zenhack commented May 27, 2023

Alright, fixed the failing tests.

@zenhack zenhack requested a review from lthibault May 27, 2023 20:26
@zenhack zenhack changed the title WIP: Add a Steal() method to Client and ClientSnapshot Add a Steal() method to Client and ClientSnapshot May 27, 2023
Otherwise the target message steals the caps, resulting in errors
later when we try to read them.
@zenhack
Copy link
Contributor Author

zenhack commented May 27, 2023

Just noticed that the .AddRef() in localpromise.go actually introduces a leak... I'm somewhat inclined to punt for now though, because the way the code works one of those Fulfills wants to be borrowed, and #480 gets rid of one of them anyway, which will make this much easier to fix.

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.

2 participants