-
Notifications
You must be signed in to change notification settings - Fork 14
Refactor grate execution path and add grate test examples #472
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
base: main
Are you sure you want to change the base?
Conversation
End-to-End Test ReportTest PreviewTest Report Deterministic TestsSummary
Test Results by Category
Non Deterministic TestsSummary
Test Results by Category
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
This looks good from my first pass. I'm tagging Justin on this one since this probably needs his final review. |
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.
I really still don't understand the closure part, but the rest seems fine. I am about to get on a flight and so will follow up when I have time on that front.
| use sysdefs::constants::{ | ||
| MAP_ANONYMOUS, MAP_PRIVATE, PAGESHIFT, PAGESIZE, PROT_EXEC, PROT_NONE, PROT_READ, PROT_WRITE, | ||
| }; // Used in `copy_data_between_cages` | ||
| use sysdefs::constants::{PROT_READ, PROT_WRITE}; // Used in `copy_data_between_cages` |
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.
This should likely be hidden with a helper function exposed by vmmap to tell you what you really want to know (whether a range is accessible).
|
I will say that I did think that putting a reference to the data you're
putting in the closure in a table indexed by thread ID should remove the
need for the closure, right? What am I missing?
…On Wed, Oct 15, 2025 at 6:02 PM Nicholas Renner ***@***.***> wrote:
*rennergade* left a comment (Lind-Project/lind-wasm#472)
<#472 (comment)>
This looks good from my first pass. I'm tagging Justin on this one since
this probably needs his final review.
—
Reply to this email directly, view it on GitHub
<#472 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGROD4A2XTZBWUCRDNHYTD3X277LAVCNFSM6AAAAACJJRPZR2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIMBYGQ3TAMBSG4>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
I think one possible solution is:
I’ll give this approach a try if that sounds reasonable. |
Co-authored-by: Justin Cappos <[email protected]>
End-to-End Test ReportTest PreviewTest Report Deterministic TestsSummary
Test Results by Category
Non Deterministic TestsSummary
Test Results by Category
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
End-to-End Test ReportTest PreviewTest Report Deterministic TestsSummary
Test Results by Category
Non Deterministic TestsSummary
Test Results by Category
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This PR refactors grate execution flow mentioned in issue #442.
Previously, the Wasm instance creation logic passed the closure directly to 3i, which then performed registration separately. In this PR, the closure is now passed as an argument to
register_handler(), allowing 3i to perform both the registration and runtime binding internally.This change:
More details in code comments
Add grate tests examples as described in issue #468