-
Notifications
You must be signed in to change notification settings - Fork 12
wasmtime - asyncify & signal interruption #349
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
This is failing some tests, do we think its related to this PR: #350 Also need to fix for linter |
arg6, | ||
), | ||
_ => { | ||
// if we are reaching here at rewind state, that means fork is called within |
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 comment could be a bit more verbose, im not sure whats going on here exactly
arg6, | ||
); | ||
|
||
// Assumption: lind_syscall_api will not switch asyncify state, which holds true for now |
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.
if we have an assumption like this we need to explain why its true
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.
a little more commenting would help, also a better PR description
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.
Would be great to have a more descriptive description, which explains why we need this modification
signal_asyncify_data: Vec<SignalAsyncifyData>, | ||
signal_asyncify_counter: u64, | ||
// syscall asyncify data, holds the sequence of syscall return value | ||
syscall_asyncify_data: Vec<i32>, |
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.
Maybe naming as sth like sys_return_async would be clearer?
assert_eq!(tank.get_fuel(), 0); | ||
} | ||
} | ||
} |
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.
Add new line
This PR is splited from PR #164
This PR focuses on spliting out the signal interruption implementation on wasmtime part. Added logic to have Asyncify supporting signal interruption on syscall