Feature/adding missing instance methods#623
Open
teleyos wants to merge 3 commits into
Open
Conversation
The `handle` param doc only said "the path of the function to retrieve",
leaving the accepted name formats to guesswork. Spell them out: plain
kebab-case export names, fully-qualified interface names including
namespace, package and version (e.g. "ns:pkg/iface@0.1.0"), and
canonical-ABI mangled resource function names ("[constructor]r",
"[method]r.f", "[static]r.f"), plus an example combining interface and
resource-constructor lookup.
Instance previously only exposed #get_func, making it impossible to fully drive a component that exports a WIT `resource` (e.g. a constructor plus instance methods) from Ruby: there was no way to look up the resource's exported type, and Val::Resource/Type::Own/Type::Borrow conversions were hard `not_implemented!` stubs. - Instance#get_resource wraps wasmtime's method of the same name; the shared lookup helper now internally prefers get_export_index over get_export since the ComponentItem the latter returns is never used. - New Wasmtime::Component::Resource wraps ResourceAny + its owning Store, requiring an explicit #resource_drop (wasmtime requires every ResourceAny, even borrows, to be explicitly dropped; there's no safe GC finalizer hook for re-entering a Store during collection). - New Wasmtime::Component::ResourceType wraps the matching wasmtime type; it is inert Copy data with no Store dependency, so unlike Resource it needs no GC mark. - convert.rs now lifts/lowers resource values for the Func::invoke path (calling exported functions), and guards against passing a Resource obtained from one Store into a call on a different Store, which would otherwise silently index into the wrong store's resource table. - Host-defined import functions (Linker#root.func_new) still don't support resources in either direction; that path never had a Ruby-visible Store handle to thread through, and stays out of scope here.
saulecabrera
reviewed
Jul 6, 2026
saulecabrera
left a comment
Member
There was a problem hiding this comment.
Looks reasonable to me. Left some comments inline.
Comment on lines
+58
to
+62
| /// Passing an +own<T>+ {Resource} into a function parameter transfers | ||
| /// ownership of the underlying resource to the callee. Using the same | ||
| /// {Resource} object afterwards (including calling {#resource_drop} on it) | ||
| /// may then raise, since the store-side handle it pointed to no longer | ||
| /// belongs to the host. |
Member
There was a problem hiding this comment.
Suggested change
| /// Passing an +own<T>+ {Resource} into a function parameter transfers | |
| /// ownership of the underlying resource to the callee. Using the same | |
| /// {Resource} object afterwards (including calling {#resource_drop} on it) | |
| /// may then raise, since the store-side handle it pointed to no longer | |
| /// belongs to the host. | |
| /// Passing an +own<T>+ {Resource} into a function transfers ownership of the | |
| /// underlying resource to the callee. Once the call returns, this {Resource} | |
| /// object is a dangling handle; it still exists, but the store-side resource | |
| /// it referred to now belongs to the guest. Calling {#resource_drop} on it, or | |
| /// passing it into another call, may then raise. (+borrow<T>+ parameters do not | |
| /// transfer ownership, the same {Resource} stays valid and reusable.) | |
Member
There was a problem hiding this comment.
Can we consider tightening the documentation here? Specifically to be more clear around the when is the resource no longer valid: I find the usage of "afterwards" to be ambiguous in this context.
Member
There was a problem hiding this comment.
Also, from what I can see there is no test covering this case? Can you add a test that covers it?
Comment on lines
+300
to
+308
| Some(current_store) | ||
| if resource.store().as_value().as_raw() | ||
| == current_store.as_value().as_raw() => | ||
| { | ||
| Ok(Val::Resource(resource.get()?)) | ||
| } | ||
| Some(_) => { | ||
| err!("Resource belongs to a different Store than the one being called into") | ||
| } |
Member
There was a problem hiding this comment.
Can we fold in the error case (when the stores differ) in the first branch?. This is minor, but I think it shows the error case clearer.
- Clarify that an own<T> Resource becomes a dangling handle after a call transfers ownership. - Refactor the Store mismatch branch in rb_to_component_val to be clearer. - Add a test covering the dangling-handle behavior after an own<T> transfer.
Member
|
LGTM. We can land this after solving the conflicts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR aims to enable users of the gem to use resources and to improve the get_func docs to talk about the canonical way to point to a function [ "packagename/interface@x.x.x" , "[constructor]my-resource" ].
One caveat is that the GC won't free a resource in order to avoid double freeing or freeing before intended so we have to free by hand using resource drop.
Example :
i made this after #619
_store became store because the store parameter went from unused to used: resource lift/lower in convert.rs now needs it to wrap returned Val::Resource values with their owning store and to reject resources passed into calls on a different store.