From b7ef5937444c2693809f149acbb57457d75fe1bb Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Tue, 19 Nov 2019 21:34:44 +0100
Subject: buffer: release buffers with free callbacks on env exit

Invoke the free callback for a given `Buffer` if it was created
with one, and mark the underlying `ArrayBuffer` as detached.

This makes sure that the memory is released e.g. when addons inside
Workers create such `Buffer`s.

PR-URL: https://github.com/nodejs/node/pull/30551
Reviewed-By: Gabriel Schulhof <gabriel.schulhof@intel.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
---
 test/addons/worker-buffer-callback/binding.cc      | 11 +++++++-
 .../worker-buffer-callback/test-free-called.js     | 17 ++++++++++++
 test/cctest/test_environment.cc                    | 32 ++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 test/addons/worker-buffer-callback/test-free-called.js

(limited to 'test')

diff --git a/test/addons/worker-buffer-callback/binding.cc b/test/addons/worker-buffer-callback/binding.cc
index a40876ebb5..1141c8a051 100644
--- a/test/addons/worker-buffer-callback/binding.cc
+++ b/test/addons/worker-buffer-callback/binding.cc
@@ -3,17 +3,24 @@
 #include <v8.h>
 
 using v8::Context;
+using v8::FunctionCallbackInfo;
 using v8::Isolate;
 using v8::Local;
 using v8::Object;
 using v8::Value;
 
+uint32_t free_call_count = 0;
 char data[] = "hello";
 
+void GetFreeCallCount(const FunctionCallbackInfo<Value>& args) {
+  args.GetReturnValue().Set(free_call_count);
+}
+
 void Initialize(Local<Object> exports,
                 Local<Value> module,
                 Local<Context> context) {
   Isolate* isolate = context->GetIsolate();
+  NODE_SET_METHOD(exports, "getFreeCallCount", GetFreeCallCount);
   exports->Set(context,
                v8::String::NewFromUtf8(
                    isolate, "buffer", v8::NewStringType::kNormal)
@@ -22,7 +29,9 @@ void Initialize(Local<Object> exports,
                    isolate,
                    data,
                    sizeof(data),
-                   [](char* data, void* hint) {},
+                   [](char* data, void* hint) {
+                     free_call_count++;
+                   },
                    nullptr).ToLocalChecked()).Check();
 }
 
diff --git a/test/addons/worker-buffer-callback/test-free-called.js b/test/addons/worker-buffer-callback/test-free-called.js
new file mode 100644
index 0000000000..2a3cc9e47c
--- /dev/null
+++ b/test/addons/worker-buffer-callback/test-free-called.js
@@ -0,0 +1,17 @@
+'use strict';
+const common = require('../../common');
+const path = require('path');
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
+const { getFreeCallCount } = require(binding);
+
+// Test that buffers allocated with a free callback through our APIs are
+// released when a Worker owning it exits.
+
+const w = new Worker(`require(${JSON.stringify(binding)})`, { eval: true });
+
+assert.strictEqual(getFreeCallCount(), 0);
+w.on('exit', common.mustCall(() => {
+  assert.strictEqual(getFreeCallCount(), 1);
+}));
diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
index 0db2963acc..132f7b44f7 100644
--- a/test/cctest/test_environment.cc
+++ b/test/cctest/test_environment.cc
@@ -1,3 +1,4 @@
+#include "node_buffer.h"
 #include "node_internals.h"
 #include "libplatform/libplatform.h"
 
@@ -208,3 +209,34 @@ TEST_F(EnvironmentTest, SetImmediateCleanup) {
   EXPECT_EQ(called, 1);
   EXPECT_EQ(called_unref, 0);
 }
+
+static char hello[] = "hello";
+
+TEST_F(EnvironmentTest, BufferWithFreeCallbackIsDetached) {
+  // Test that a Buffer allocated with a free callback is detached after
+  // its callback has been called.
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+
+  int callback_calls = 0;
+
+  v8::Local<v8::ArrayBuffer> ab;
+  {
+    Env env {handle_scope, argv};
+    v8::Local<v8::Object> buf_obj = node::Buffer::New(
+        isolate_,
+        hello,
+        sizeof(hello),
+        [](char* data, void* hint) {
+          CHECK_EQ(data, hello);
+          ++*static_cast<int*>(hint);
+        },
+        &callback_calls).ToLocalChecked();
+    CHECK(buf_obj->IsUint8Array());
+    ab = buf_obj.As<v8::Uint8Array>()->Buffer();
+    CHECK_EQ(ab->ByteLength(), sizeof(hello));
+  }
+
+  CHECK_EQ(callback_calls, 1);
+  CHECK_EQ(ab->ByteLength(), 0);
+}
-- 
cgit v1.2.3