hyperlight-dev / hyperlight-dev/hyperlight-wasm

Rethink state management and option semnatics of proto/wasm/loaded sandbox

Open
#136 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

lifecycle/confirmed
Dominant language
Rust
Stars
728
Forks
39
Avg merge
2d 6h
Merged PRs (30d)
23

Description

This is taking self by value, so why do we need to move out of these fields? Is it just about the fact that we have drop glue on this type?

One option to get rid of the dynamic check here would be ManuallyDrop, but I don't know if we like the tradeoff of (closer to what we actually want, no unnecessary dynamic checks) to increased unsafe code surface, and I suppose we would still need to have a flag that let the Drop impl know whether or not the contents had been moved (or use ManuallyDrop<Self> to write an into_inner() function that removes the contents of the sandbox, manually updates the counters, and never runs the drop glue at all).

Relatedly, I think that scattered throughout our errors in general, we have a mix of "this means the user did something wrong, and they should be better" with "this means this is a fundamental invariant that we believed would always be true but was somehow violated". I would like us to separate these in an easily visible way (akin to rustc errors vs ICEs), since they have quite different consequences. I wonder if it is reasonable to panic in the latter case, since if we were wrong about one invariant, who knows what else we were wrong about...

Sorry, this comment has become a bit long and large in scope, so feel free to not consider it a blocker for this PR, but perhaps think a little bit about whether the Option semantics are indeed what we want.

Originally posted by @syntactically in https://github.com/hyperlight-dev/hyperlight-wasm/pull/128#discussion_r2239536449

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the proto/wasm/loaded sandbox state-management discussion and the related PR 128 thread. Determine the intended Option and drop semantics, then separate user-facing errors from invariant violations; done means the design is agreed and its consequences are documented or implemented.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust, wasm
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.