[GIT PULL] chnet updates

[GIT PULL] chnet updates

[Ammar Faizi]

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!

The following changes since commit 63fcbde6002bbd77615b5e0c6092ee3cc01ed53d:

   doc: chnet: Start documenting ring object (2022-09-13 15:28:34 +0700)

are available in the Git repository at:

   https://gitlab.torproject.org/ammarfaizi2/ncns.git tags/chnet.2022-10-03

for you to fetch changes up to 75d1566beb3c55711ce79df6734433e286830f13:

   tests/js/rings: Remove endless loop test (2022-10-03 18:44:07 +0700)

----------------------------------------------------------------
chnet.2022-10-03

----------------------------------------------------------------
Ammar Faizi (31):
     chnet: node: Add NodeJS integration chunked request body
     tests/js/ring: Enable all tests
     Merge branch 'dev.http_body_chunked' (chunked HTTP request body feature)
     chnet: Prepare global struct ch_thpool array
     chnet: Implement `get_thread()` and `put_thread()` function
     chnet: Collect response headers in an `std::vector` container
     chnet: Add `GetResponseHeaders()` method
     Merge branch 'dev.get_res_hdr_v4'
     chnet: Refactor response header function
     tests/cpp/ring: Add test for get response header function
     chnet: node: Implement get response headers functions
     Merge branch 'dev.documentation' into tmp.master
     Merge branch 'dev.fixup_res_hdr' into tmp.master
     Delete scripts/main.js
     tests/js/ring: Add test_chnet_ring_simple_post()
     tests/js/ring: Add test_chnet_ring_simple_post_parallel()
     Merge branch 'dev.req_body_unit_tests' (Add missing NodeJS unit tests)
     Merge branch 'dev.documentation' (Start documenting ring object)
     Delete design.txt (it's not longer used)
     chnet: node: Destroy `__udata` key in the `Close()` call
     Merge branch 'dev.next' (fixup memory leak and cleanup)
     chromium: Update chromium submodule
     chromium: Add chromium specific file
     chnet: WorkQueue: Fix clang error about shadowing a field
     chnet: boringssl: Turn off boring SSL patch
     node-addon-api: Update Napi module
     Merge branch 'dev.hotfix' (small hot fixes)
     chnet: node: Completely redesign CHNet ring
     Merge branch 'dev.ptr_obj_v2' (Completely redesign CHNet ring)
     chnet: node: Delete old file chnet/chnet_node.old.cc
     tests/js/rings: Remove endless loop test

  chnet/WorkQueue.h                       |   4 +-
  chnet/chnet.cc                          | 182 +++++++-
  chnet/chnet.h                           |  40 +-
  chnet/chnet_node.cc                     | 913 ++++++++++++++++++++++------------------
  chnet/fixup.cc                          |   6 +
  chromium/.gclient_previous_sync_commits |   1 +
  chromium/src                            |   2 +-
  design.txt                              |  17 -
  node-addon-api                          |   2 +-
  scripts/main.js                         |  32 --
  tests/cpp/ring.cc                       |  24 ++
  tests/js/ring.js                        | 367 +++++++++++++++-
  12 files changed, 1116 insertions(+), 474 deletions(-)
  create mode 100644 chromium/.gclient_previous_sync_commits
  delete mode 100644 design.txt
  delete mode 100644 scripts/main.js

-- 
Ammar Faizi

Re: [GIT PULL] chnet updates

[Alviro Iskandar Setiawan]

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