From 809386aa1193ae96a94af635965ae2beff67373f Mon Sep 17 00:00:00 2001
From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
Date: Tue, 12 Apr 2022 02:16:57 +0800
Subject: [PATCH 1/3] src: fix memory leak for v8.serialize

When Buffer::New passes in existing data,
it cannot be garbage collected in js synchronous execution.

Fixes: https://github.com/nodejs/node/issues/40828
Refs: https://github.com/nodejs/node/issues/38300
---
 src/node_serdes.cc                      |  3 ++-
 test/parallel/test-v8-serialize-leak.js | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-v8-serialize-leak.js

diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index f6f0034bc24d09..9840f1a5f06718 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -209,9 +209,10 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
   // Note: Both ValueSerializer and this Buffer::New() variant use malloc()
   // as the underlying allocator.
   std::pair<uint8_t*, size_t> ret = ctx->serializer_.Release();
-  auto buf = Buffer::New(ctx->env(),
+  auto buf = Buffer::Copy(ctx->env(),
                          reinterpret_cast<char*>(ret.first),
                          ret.second);
+  free(ret.first);
 
   if (!buf.IsEmpty()) {
     args.GetReturnValue().Set(buf.ToLocalChecked());
diff --git a/test/parallel/test-v8-serialize-leak.js b/test/parallel/test-v8-serialize-leak.js
new file mode 100644
index 00000000000000..4719929709b969
--- /dev/null
+++ b/test/parallel/test-v8-serialize-leak.js
@@ -0,0 +1,22 @@
+'use strict';
+// Flags: --expose-gc
+
+require('../common');
+const v8 = require('v8');
+const assert = require('assert');
+
+const before = process.memoryUsage.rss();
+
+for (let i = 0; i < 1000000; i++) {
+  v8.serialize('');
+}
+
+global.gc();
+
+const after = process.memoryUsage.rss();
+
+if (process.config.variables.asan) {
+  assert(before * 10 > after, `asan: before=${before} after=${after}`);
+} else {
+  assert(after - before < 1024 * 1024 * 10, `before=${before} after=${after}`);
+}

From 806687b304e414cb767d855a9fb79aa93cbe30ea Mon Sep 17 00:00:00 2001
From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
Date: Wed, 13 Apr 2022 19:37:11 +0800
Subject: [PATCH 2/3] use a better way

---
 src/node_buffer.cc | 16 ++++++++++++++--
 src/node_serdes.cc |  3 +--
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
index 215bd8003aabe1..dcf5d84ca34a2e 100644
--- a/src/node_buffer.cc
+++ b/src/node_buffer.cc
@@ -502,8 +502,20 @@ MaybeLocal<Object> New(Environment* env,
     }
   }
 
-  auto free_callback = [](char* data, void* hint) { free(data); };
-  return New(env, data, length, free_callback, nullptr);
+  EscapableHandleScope handle_scope(env->isolate());
+
+  auto free_callback = [](void* data, size_t length, void* deleter_data) {
+    free(data);
+  };
+  std::unique_ptr<BackingStore> bs =
+      v8::ArrayBuffer::NewBackingStore(data, length, free_callback, nullptr);
+
+  Local<ArrayBuffer> ab = v8::ArrayBuffer::New(env->isolate(), std::move(bs));
+
+  Local<Object> obj;
+  if (Buffer::New(env, ab, 0, length).ToLocal(&obj))
+    return handle_scope.Escape(obj);
+  return Local<Object>();
 }
 
 namespace {
diff --git a/src/node_serdes.cc b/src/node_serdes.cc
index 9840f1a5f06718..f6f0034bc24d09 100644
--- a/src/node_serdes.cc
+++ b/src/node_serdes.cc
@@ -209,10 +209,9 @@ void SerializerContext::ReleaseBuffer(const FunctionCallbackInfo<Value>& args) {
   // Note: Both ValueSerializer and this Buffer::New() variant use malloc()
   // as the underlying allocator.
   std::pair<uint8_t*, size_t> ret = ctx->serializer_.Release();
-  auto buf = Buffer::Copy(ctx->env(),
+  auto buf = Buffer::New(ctx->env(),
                          reinterpret_cast<char*>(ret.first),
                          ret.second);
-  free(ret.first);
 
   if (!buf.IsEmpty()) {
     args.GetReturnValue().Set(buf.ToLocalChecked());

From 74a59561b0327c3631cfab76b8888c8bb158aade Mon Sep 17 00:00:00 2001
From: liuxingbaoyu <30521560+liuxingbaoyu@users.noreply.github.com>
Date: Wed, 13 Apr 2022 20:45:13 +0800
Subject: [PATCH 3/3] fix test

---
 test/parallel/test-v8-serialize-leak.js | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/parallel/test-v8-serialize-leak.js b/test/parallel/test-v8-serialize-leak.js
index 4719929709b969..4e3a1d96f8ad6e 100644
--- a/test/parallel/test-v8-serialize-leak.js
+++ b/test/parallel/test-v8-serialize-leak.js
@@ -16,7 +16,7 @@ global.gc();
 const after = process.memoryUsage.rss();
 
 if (process.config.variables.asan) {
-  assert(before * 10 > after, `asan: before=${before} after=${after}`);
+  assert(after < before * 10, `asan: before=${before} after=${after}`);
 } else {
-  assert(after - before < 1024 * 1024 * 10, `before=${before} after=${after}`);
+  assert(after < before * 2, `before=${before} after=${after}`);
 }