MiniV8 (interface wrapper around OwnedIsolate) memory leak

[AlaaAlHallaq]

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 drop OwnedIsolate even after I dropped the MiniV8 instance; this only happens after using MiniV8::create_function.

[SkylerLipthay]

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.

[AlaaAlHallaq]

@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 !!)

[SkylerLipthay]

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.

[AlaaAlHallaq]

@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.
• remove the stack of 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.

[AlaaAlHallaq]

@SkylerLipthay I would love to take your notes on the pr.

[SkylerLipthay]

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.

[AlaaAlHallaq]

@SkylerLipthay would you consider merging the pr or at least review it, or would on fixing the issue ?