-
Notifications
You must be signed in to change notification settings - Fork 491
add gc support to Rust owned objects #5638
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
0f0adb0 to
fa9ad80
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
guybedford
left a comment
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.
Lovely work!
| /// When a resource is wrapped for JavaScript, `State` stores a strong V8 Global handle to the | ||
| /// wrapper object. The `strong_ref_count` tracks how many Rust `Ref<R>` handles exist. When the | ||
| /// count reaches zero and a wrapper exists, the Global is made weak so V8 can garbage collect | ||
| /// the wrapper and trigger cleanup via the weak callback. |
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.
Neat!
| } | ||
| } | ||
|
|
||
| pub fn unwrap<'a, R: Resource>(lock: &'a mut Lock, value: v8::Local<v8::Value>) -> &'a mut R { |
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.
It's a bit of a concern from a Rust perspective if you can obtain multiple mutable borrows this way, should we protect against that case?
|
|
||
| impl<T: Resource> DerefMut for Ref<T> { | ||
| fn deref_mut(&mut self) -> &mut Self::Target { | ||
| unsafe { &mut (*self.instance).resource } |
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.
It seems like this permits multiple mut refs to the same data, do we need to consider protections?
This PR adds garbage collection support to Rust-owned JSG resources, enabling proper cleanup when V8 collects JavaScript wrapper objects.