Skip to content

CABI: Add a resource.consume method for owned handles #238

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

Open
juntyr opened this issue Aug 22, 2023 · 2 comments
Open

CABI: Add a resource.consume method for owned handles #238

juntyr opened this issue Aug 22, 2023 · 2 comments

Comments

@juntyr
Copy link

juntyr commented Aug 22, 2023

Motivation

At the moment, there is no way to convert an owned resource handle inside the resource-exporting component back into an owned value of the type that implements the resource. While the specifics of performing this conversion can be left to wit-bindgen (see, e.g., bytecodealliance/wit-bindgen#641 (comment)), it requires a canonical ABI primitive to consume an owned resource handle.

Detail

The canonical ABI defines a new method, resource.consume, which might be implemented as follows:

### `canon resource.consume`

def canon_resource_consume(inst, rt, i):
  # similar to resource.drop, but only for owned handles
  h = inst.handles.remove(rt, i)
  trap_if(!h.own)
  assert(h.scope is None)
  trap_if(h.lend_count != 0)
  trap_if(inst is not rt.impl and not rt.impl.may_enter)

  # note: do *not* call h's destructor

  # similar to resource.rep
  return h.rep

In particular, resource.consume, which has a (handwavy) signature of fn(owned_handle: i32) -> isize:

  • takes an owned handle
  • deallocates the handle, like resource.drop
  • does not call the resource's destructor, since this method may be used to reobtain ownership of the resource's implementing value
  • returns the handle's representation, like resource.rep

Closing Notes

Thank you for considering this extension to the canonical ABI :)

@lukewagner
Copy link
Member

Good idea, and thanks for the super-clear write-up! I suppose this is technically possible to achieve today by adding a branch to the destructor to make it a no-op in the cases where you'd use resource.consume, but that has some overhead and could be a real pain, possibly requiring changing the linear-memory representation or adding an indirection, all of which is avoided by resource.consume. Does that sound right?

@juntyr
Copy link
Author

juntyr commented Aug 23, 2023

Thanks! You’re absolutely right that the intended outcome can be achieved already, though with some tricks. In Rust, for instance,

  • the resource type can contain an Option<T>, where T is the “real” type that provides the resource’s functionality. Consuming is safe with the Option::take method, but every deref access requires conditional checks for None. I am using this approach myself right now, as it minimised user-side unsafety.
  • the resource type can contain a ManuallyDrop<T> and a boolean “should I drop this” flag. Consuming the resource value requires the unsafe ManuallyDrop::take static method. Similarly, the destructor is now unsafe as it needs to conditionally call the unsafe ManuallyDrop::drop method if the flag is true. The advantage of this approach is that it limits unsafety to the actual places where the assumption that the inner value is only ever dropped once is made. While I don’t think that this approach should be recommended to end users, it is one that e.g. wit-bindgen could use internally to implement consuming a resource without any CABI support.
  • alternatively, wit-bindgen could also internally use an unsafe combination of the two above approaches, where an Option<T> is used to get any potential niche memory layout optimisations, but unsafe code is used in every deref and the final consume to assume that the option is Some(t). While it has the potential advantage of a lower memory footprint, it also moves the unsafety to the wrong method (deref is always safe since having a handle already guarantees that the resource still exists).

Even though the last two options presents reasonable workarounds that still present a safe interface to users, I still think that an explicit CABI function for invalidating an owned handle without dropping the inner value is a better approach. It avoids both the potential memory overhead and additional unsafety.

Overall, I fully agree with your assessment.

dicej added a commit to dicej/wit-bindgen that referenced this issue Nov 10, 2023
This allows the guest to take back ownership of an exported resource from the
host; you can think of it as the reverse of `Resource::new`.  It's a bit awkward
to do this for the time being;
WebAssembly/component-model#238 will improve the
situation if accepted.

Signed-off-by: Joel Dice <[email protected]>
alexcrichton pushed a commit to bytecodealliance/wit-bindgen that referenced this issue Nov 13, 2023
* add `Resource::take` method to `wit_bindgen::rt`

This allows the guest to take back ownership of an exported resource from the
host; you can think of it as the reverse of `Resource::new`.  It's a bit awkward
to do this for the time being;
WebAssembly/component-model#238 will improve the
situation if accepted.

Signed-off-by: Joel Dice <[email protected]>

* address review feedback

- add `type RawRep<T> = Option<T>` alias
- rename `Resource::take` to `Resource::into_inner`
- add `Resource::lift_borrow` and use it in code generator

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
rvolosatovs pushed a commit to bytecodealliance/wrpc that referenced this issue May 23, 2024
* add `Resource::take` method to `wit_bindgen::rt`

This allows the guest to take back ownership of an exported resource from the
host; you can think of it as the reverse of `Resource::new`.  It's a bit awkward
to do this for the time being;
WebAssembly/component-model#238 will improve the
situation if accepted.

Signed-off-by: Joel Dice <[email protected]>

* address review feedback

- add `type RawRep<T> = Option<T>` alias
- rename `Resource::take` to `Resource::into_inner`
- add `Resource::lift_borrow` and use it in code generator

Signed-off-by: Joel Dice <[email protected]>

---------

Signed-off-by: Joel Dice <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants