Skip to content

Improve the ergonomics of registering host functions #468

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

Merged
merged 1 commit into from
May 8, 2025

Conversation

jprendes
Copy link
Contributor

@jprendes jprendes commented May 7, 2025

This PR adds the register and register_with_extra_allowed_syscalls methods to the UninitializedSandbox to register host methods.
e.g.,

uninitialized_sandbox.register("add", |a: i32, b: i32| {
    Ok(a + b)
})?;

This method accepts either:

  • FnMut(..) -> Result<R>
  • Arc<Mutex<FnMut(..) -> Result<R>>>
  • &Arc<Mutex<FnMut(..) -> Result<R>>>

This is controlled by the IntoHostFunction trait.

This is not a breaking change. It's an additive change, and the previous registration mechanism continues to works.

@jprendes jprendes requested review from danbugs and ludfjig May 7, 2025 19:18
@jprendes jprendes added kind/enhancement For PRs adding features, improving functionality, docs, tests, etc. area/API Related to the API or public interface labels May 7, 2025
ludfjig
ludfjig previously approved these changes May 7, 2025
Copy link
Contributor

@ludfjig ludfjig left a comment

Choose a reason for hiding this comment

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

lgtm!

Copy link
Contributor

@danbugs danbugs left a comment

Choose a reason for hiding this comment

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

LGTM. Wonder if we should just remove the previous registration mechanism...

@jprendes
Copy link
Contributor Author

jprendes commented May 7, 2025

Wonder if we should just remove the previous registration mechanism...

The new mechanism is a wrapper around the old one. I would need to think how we can stop exposing the old mechanism without violating some "X is more private than Y" rule.

@jprendes jprendes force-pushed the host_function_2 branch from 5b6020f to 0d443c2 Compare May 7, 2025 21:21
@jprendes jprendes requested review from danbugs and ludfjig May 7, 2025 21:24
@jprendes jprendes force-pushed the host_function_2 branch from 0d443c2 to 7561f6f Compare May 8, 2025 05:22
@jprendes jprendes enabled auto-merge (squash) May 8, 2025 05:23
@jprendes jprendes merged commit 5cd594e into hyperlight-dev:main May 8, 2025
27 checks passed
@jprendes jprendes deleted the host_function_2 branch May 8, 2025 06:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API Related to the API or public interface kind/enhancement For PRs adding features, improving functionality, docs, tests, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants