Re: [GIT PULL] chnet updates

[Alviro Iskandar Setiawan <alviro.iskandar@gnuweeb.org>]

On Mon, Oct 3, 2022 at 6:55 PM Ammar Faizi wrote:
> Hi Al,
>
> Various chnet updates. Mostly just general updates, the most noticable
> change is the recent memory leak bug fix.
>
> The previous Node JS ring wrapper design has a memory leak issue. It
> is not clear how to avoid memory leak issue when we are attaching a
> function or a class with user data to a Napi::Object. I have reported
> the issue to the Node JS team. They said they are going to fix it from
> the Node JS core side.
>
> Link: https://github.com/nodejs/node-addon-api/issues/1213
> Link: https://github.com/nodejs/node-addon-api/issues/1212
>
> In short, we can't rely on the GC calling the registered finalizers.
>
> For now, redesign the Node JS wrapper to avoid this issue. Use
> a Napi::Number to save the C++ pointer and pass the number as a Node
> JS function argument, then internally cast the number back to C++
> pointer each function call that requires a CHNet pointer interaction.
>
> This design is unsafe, because if the given number argument is not a
> valid pointer, it will lead to undefined behavior. Create a class
> wrapper written in JavaScript to make it safer. The class wrapper also
> adapts the previous object design to minimize breaking changes.
>
> This has been tested with the current unit tests and the result is
> all green.
>
> Please pull!

very good, pulled

tq

-- Viro