Open AlaaAlHallaq opened 3 weeks ago
Thank you for the kind words. Could you write up a minimal reproduction of this issue? Is it perhaps as simple as the following?
let mv8 = MiniV8::new();
let func = mv8.create_function(|inv| ...);
drop(mv8);
I see a TODO related to memory management in create_function
, but I believe this should only be the cause of deferred garbage collection, not outright leakage.
@SkylerLipthay thanks for the immediate response;
I think the issue is not related to the comment about v8::Isolate::adjust_amount_of_external_allocated_memory
;
I think its related to retaining the Mini8 reference inside the the v8::Function
external data causing a retain cycle.
what I may propose the following:
create a shallow wrapper of MiniV8 around the provided v8::HandleScope
provided by v8 function callback to be used as a regular MiniV8 instance within the callback, I also think that would remove the need to store a stack of v8::HandleScope
within MiniV8::interface
.
Note in the original issue, if I manually called MiniV8::interface::pop (witch will force dropping OwnedIsolate
& free every thing !!)
I have to become re-familiarized with my own code here, it's been a while. It does seem that the scope stack design is excessive. Even if you force drop the OwnedIsolate
, is there still a minor memory leak related of the emptied Interface
?
When I have more time, I can reproduce the leak and try to come up with a better design. I still have not wrapped my head around the retain cycle issue, which seems to be the fundamental problem. It seems like I was confident with my usage of v8::Weak::with_guaranteed_finalizer
, but I must be wrong somewhere.
Meanwhile, I'd greatly appreciate a PR or more details/reproduction, if you have time. Thank you for digging into the code.
@SkylerLipthay that's the best I can do for now.. note: the following can be done:
CallbackInfo
struct can be removed & replace it's usage with Callback
aliase.InterfaceEntry
from MiniV8 & only store a Rc<RefCell<InterfaceEntry>>
.
Note: I see no issue updating v8 dep to the latest version while you are at it.@SkylerLipthay I would love to take your notes on the pr.
Thank you for the PR, this is what I was hoping for. I appreciate the reproduction notes, too. I'll work on this within the next day and hopefully have a quick resolution and new crate version.
I agree this would be a good time to update v8
.
@SkylerLipthay would you consider merging the pr or at least review it, or would on fixing the issue ?
currently I am trying to embed v8 into my application and you lib provided the most simple ABI, so thanks for the effort.
but in my case I noticed a memory leak tried to debug it & found that
Interface
does not dropOwnedIsolate
even after I dropped theMiniV8
instance; this only happens after usingMiniV8::create_function
.