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

Throw, instead of assert, for transferring detached ArrayBuffers #1420

Merged
merged 1 commit into from
Jul 19, 2024

Conversation

domenic
Copy link
Member

@domenic domenic commented Jul 18, 2024

This makes the algorithm more user-friendly, as this is generally the desired behavior, and the algorithm can already throw for other reasons so callers are expected to handle that.

/cc @inexorabletash as this was inspired by your work in #1419.

I audited the callers and they are all fine with this. (Most are just referencing the concept of transferring, as these definitions are not yet widely used.)


Preview | Diff

This makes the algorithm more user-friendly, as this is generally the desired behavior, and the algorithm can already throw for other reasons so callers are expected to handle that.
@domenic domenic requested a review from annevk July 18, 2024 01:23
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

This seems fine, but this algorithm seems to imply that allocation in the target realm can never throw. That seems unclear? Perhaps if the realms are in the same agent cluster, but even then.

@domenic
Copy link
Member Author

domenic commented Jul 19, 2024

I think you're right that this algorithm assumes same agent-cluster. Let me fix that in a separate PR.

@domenic domenic merged commit 155ae10 into main Jul 19, 2024
2 checks passed
@domenic domenic deleted the throw-on-transfer branch July 19, 2024 01:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants