fix(napi): avoid add finalizer if properties is empty - #2711
Conversation
| .map(|p| p.data) | ||
| .filter(|data| !data.is_null()) | ||
| .collect::<Vec<*mut std::ffi::c_void>>(); | ||
| let len = Box::into_raw(Box::new(closures.len())); |
There was a problem hiding this comment.
The problem I noticed is that if you allocate this empty box there is still a leak. My try fix was to avoid this allocation altogether and also not add the finalizers if closures was empty.
Is there a requirement to add finalization regardless if you have closures or not?
There was a problem hiding this comment.
I have if !properties.is_empty() { } here, I believe it can avoid the empty Box allocation and useless napi_add_finalizer
There was a problem hiding this comment.
Yes, but properties is not empty. The closures vec is empty, because the properties added are plain name: value properties.
So my question is if you have properties, but not closures for them, if you still need to add the finalizers...
My fix is just
if closures.len() > 0 {
// alloc box
// add finalizer
}
There was a problem hiding this comment.
Oh, my bad, will create another pr to fix it
2 participants
Maybe can resolve #2708, also boost some perf