Skip to content
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

RefCell<Option<Heap<T>>> is easy to misuse #25726

Open
jdm opened this issue Feb 11, 2020 · 1 comment
Open

RefCell<Option<Heap<T>>> is easy to misuse #25726

jdm opened this issue Feb 11, 2020 · 1 comment

Comments

@jdm
Copy link
Member

@jdm jdm commented Feb 11, 2020

Code like this is subtly incorrect:

        // Cache the Js value.
        let heap_val = Heap::default();
        heap_val.set(frozen_types);
        *self.frozen_supported_performance_entry_types.borrow_mut() = Some(heap_val);

Moving around Heap values invalidates pointers; the correct way to use this is to set the Some(Heap::default()), then use borrow_mut().as_ref().unwrap().set(...) to update the value after it has settled in memory.

@jdm
Copy link
Member Author

@jdm jdm commented Feb 11, 2020

One option: build a wrapper type that supports this pattern safely (OptionalHeap<T>) and ban the Option<Heap<T>> types.

@jdm jdm mentioned this issue Feb 11, 2020
4 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.