GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — buffer: release buffers with free callbacks on env exit · nodejs/node@8252c06

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

src
- node_buffer.cc
test
- addons/worker-buffer-callback
  - binding.cc
  - test-free-called.js
- cctest
  - test_environment.cc

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎src/node_buffer.cc‎`

Copy file name to clipboardExpand all lines: src/node_buffer.cc

+36-9Lines changed: 36 additions & 9 deletions

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ using v8::Context;`
`53`	`53`	`using v8::EscapableHandleScope;`
`54`	`54`	`using v8::FunctionCallbackInfo;`
`55`	`55`	`using v8::Global;`
	`56`	`+using v8::HandleScope;`
`56`	`57`	`using v8::Int32;`
`57`	`58`	`using v8::Integer;`
`58`	`59`	`using v8::Isolate;`
`@@ -73,8 +74,10 @@ namespace {`
`73`	`74`
`74`	`75`	`class CallbackInfo {`
`75`	`76`	`public:`
	`77`	`+ ~CallbackInfo();`
	`78`	`+`
`76`	`79`	`static inline void Free(char* data, void* hint);`
`77`		`- static inline CallbackInfo* New(Isolate* isolate,`
	`80`	`+ static inline CallbackInfo* New(Environment* env,`
`78`	`81`	`Local<ArrayBuffer> object,`
`79`	`82`	`FreeCallback callback,`
`80`	`83`	`char* data,`
`@@ -84,9 +87,10 @@ class CallbackInfo {`
`84`	`87`	`CallbackInfo& operator=(const CallbackInfo&) = delete;`
`85`	`88`
`86`	`89`	`private:`
	`90`	`+ static void CleanupHook(void* data);`
`87`	`91`	`static void WeakCallback(const WeakCallbackInfo<CallbackInfo>&);`
`88`	`92`	`inline void WeakCallback(Isolate* isolate);`
`89`		`- inline CallbackInfo(Isolate* isolate,`
	`93`	`+ inline CallbackInfo(Environment* env,`
`90`	`94`	`Local<ArrayBuffer> object,`
`91`	`95`	`FreeCallback callback,`
`92`	`96`	`char* data,`
`@@ -95,6 +99,7 @@ class CallbackInfo {`
`95`	`99`	`FreeCallback const callback_;`
`96`	`100`	`char* const data_;`
`97`	`101`	`void* const hint_;`
	`102`	`+ Environment* const env_;`
`98`	`103`	`};`
`99`	`104`
`100`	`105`
`@@ -103,31 +108,53 @@ void CallbackInfo::Free(char* data, void*) {`
`103`	`108`	`}`
`104`	`109`
`105`	`110`
`106`		`-CallbackInfo* CallbackInfo::New(Isolate* isolate,`
	`111`	`+CallbackInfo* CallbackInfo::New(Environment* env,`
`107`	`112`	`Local<ArrayBuffer> object,`
`108`	`113`	`FreeCallback callback,`
`109`	`114`	`char* data,`
`110`	`115`	`void* hint) {`
`111`		`- return new CallbackInfo(isolate, object, callback, data, hint);`
	`116`	`+ return new CallbackInfo(env, object, callback, data, hint);`
`112`	`117`	`}`
`113`	`118`
`114`	`119`
`115`		`-CallbackInfo::CallbackInfo(Isolate* isolate,`
	`120`	`+CallbackInfo::CallbackInfo(Environment* env,`
`116`	`121`	`Local<ArrayBuffer> object,`
`117`	`122`	`FreeCallback callback,`
`118`	`123`	`char* data,`
`119`	`124`	`void* hint)`
`120`		`- : persistent_(isolate, object),`
	`125`	`+ : persistent_(env->isolate(), object),`
`121`	`126`	`callback_(callback),`
`122`	`127`	`data_(data),`
`123`		`- hint_(hint) {`
	`128`	`+ hint_(hint),`
	`129`	`+ env_(env) {`
`124`	`130`	`ArrayBuffer::Contents obj_c = object->GetContents();`
`125`	`131`	`CHECK_EQ(data_, static_cast<char*>(obj_c.Data()));`
`126`	`132`	`if (object->ByteLength() != 0)`
`127`	`133`	`CHECK_NOT_NULL(data_);`
`128`	`134`
`129`	`135`	`persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);`
`130`		`- isolate->AdjustAmountOfExternalAllocatedMemory(sizeof(*this));`
	`136`	`+ env->AddCleanupHook(CleanupHook, this);`
	`137`	`+ env->isolate()->AdjustAmountOfExternalAllocatedMemory(sizeof(*this));`
	`138`	`+}`
	`139`	`+`
	`140`	`+`
	`141`	`+CallbackInfo::~CallbackInfo() {`
	`142`	`+ persistent_.Reset();`
	`143`	`+ env_->RemoveCleanupHook(CleanupHook, this);`
	`144`	`+}`
	`145`	`+`
	`146`	`+`
	`147`	`+void CallbackInfo::CleanupHook(void* data) {`
	`148`	`+ CallbackInfo* self = static_cast<CallbackInfo*>(data);`
	`149`	`+`
	`150`	`+ {`
	`151`	`+ HandleScope handle_scope(self->env_->isolate());`
	`152`	`+ Local<ArrayBuffer> ab = self->persistent_.Get(self->env_->isolate());`
	`153`	`+ CHECK(!ab.IsEmpty());`
	`154`	`+ ab->Detach();`
	`155`	`+ }`
	`156`	`+`
	`157`	`+ self->WeakCallback(self->env_->isolate());`
`131`	`158`	`}`
`132`	`159`
`133`	`160`
`@@ -391,7 +418,7 @@ MaybeLocal<Object> New(Environment* env,`
`391`	`418`	`}`
`392`	`419`	`MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);`
`393`	`420`
`394`		`- CallbackInfo::New(env->isolate(), ab, callback, data, hint);`
	`421`	`+ CallbackInfo::New(env, ab, callback, data, hint);`
`395`	`422`
`396`	`423`	`if (ui.IsEmpty())`
`397`	`424`	`return MaybeLocal<Object>();`

Collapse file

`‎test/addons/worker-buffer-callback/binding.cc‎`

Copy file name to clipboardExpand all lines: test/addons/worker-buffer-callback/binding.cc

+10-1Lines changed: 10 additions & 1 deletion

Original file line number	Diff line number	Diff line change
`@@ -3,17 +3,24 @@`
`3`	`3`	`#include <v8.h>`
`4`	`4`
`5`	`5`	`using v8::Context;`
	`6`	`+using v8::FunctionCallbackInfo;`
`6`	`7`	`using v8::Isolate;`
`7`	`8`	`using v8::Local;`
`8`	`9`	`using v8::Object;`
`9`	`10`	`using v8::Value;`
`10`	`11`
	`12`	`+uint32_t free_call_count = 0;`
`11`	`13`	`char data[] = "hello";`
`12`	`14`
	`15`	`+void GetFreeCallCount(const FunctionCallbackInfo<Value>& args) {`
	`16`	`+ args.GetReturnValue().Set(free_call_count);`
	`17`	`+}`
	`18`	`+`
`13`	`19`	`void Initialize(Local<Object> exports,`
`14`	`20`	`Local<Value> module,`
`15`	`21`	`Local<Context> context) {`
`16`	`22`	`Isolate* isolate = context->GetIsolate();`
	`23`	`+ NODE_SET_METHOD(exports, "getFreeCallCount", GetFreeCallCount);`
`17`	`24`	`exports->Set(context,`
`18`	`25`	`v8::String::NewFromUtf8(`
`19`	`26`	`isolate, "buffer", v8::NewStringType::kNormal)`
`@@ -22,7 +29,9 @@ void Initialize(Local<Object> exports,`
`22`	`29`	`isolate,`
`23`	`30`	`data,`
`24`	`31`	`sizeof(data),`
`25`		`- [](char* data, void* hint) {},`
	`32`	`+ [](char* data, void* hint) {`
	`33`	`+ free_call_count++;`
	`34`	`+ },`
`26`	`35`	`nullptr).ToLocalChecked()).Check();`
`27`	`36`	`}`
`28`	`37`

Collapse file

`‎test/addons/worker-buffer-callback/test-free-called.js‎`

Copy file name to clipboard

+17Lines changed: 17 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,17 @@`
	`1`	`+'use strict';`
	`2`	`+const common = require('../../common');`
	`3`	`+const path = require('path');`
	`4`	`+const assert = require('assert');`
	`5`	`+const { Worker } = require('worker_threads');`
	`6`	+const binding = path.resolve(__dirname, `./build/${common.buildType}/binding`);
	`7`	`+const { getFreeCallCount } = require(binding);`
	`8`	`+`
	`9`	`+// Test that buffers allocated with a free callback through our APIs are`
	`10`	`+// released when a Worker owning it exits.`
	`11`	`+`
	`12`	+const w = new Worker(`require(${JSON.stringify(binding)})`, { eval: true });
	`13`	`+`
	`14`	`+assert.strictEqual(getFreeCallCount(), 0);`
	`15`	`+w.on('exit', common.mustCall(() => {`
	`16`	`+ assert.strictEqual(getFreeCallCount(), 1);`
	`17`	`+}));`

Collapse file

`‎test/cctest/test_environment.cc‎`

Copy file name to clipboardExpand all lines: test/cctest/test_environment.cc

+32Lines changed: 32 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,4 @@`
	`1`	`+#include "node_buffer.h"`
`1`	`2`	`#include "node_internals.h"`
`2`	`3`	`#include "libplatform/libplatform.h"`
`3`	`4`
`@@ -185,3 +186,34 @@ static void at_exit_js(void* arg) {`
`185`	`186`	`assert(obj->IsObject());`
`186`	`187`	`called_at_exit_js = true;`
`187`	`188`	`}`
	`189`	`+`
	`190`	`+static char hello[] = "hello";`
	`191`	`+`
	`192`	`+TEST_F(EnvironmentTest, BufferWithFreeCallbackIsDetached) {`
	`193`	`+ // Test that a Buffer allocated with a free callback is detached after`
	`194`	`+ // its callback has been called.`
	`195`	`+ const v8::HandleScope handle_scope(isolate_);`
	`196`	`+ const Argv argv;`
	`197`	`+`
	`198`	`+ int callback_calls = 0;`
	`199`	`+`
	`200`	`+ v8::Local<v8::ArrayBuffer> ab;`
	`201`	`+ {`
	`202`	`+ Env env {handle_scope, argv};`
	`203`	`+ v8::Local<v8::Object> buf_obj = node::Buffer::New(`
	`204`	`+ isolate_,`
	`205`	`+ hello,`
	`206`	`+ sizeof(hello),`
	`207`	`+ [](char* data, void* hint) {`
	`208`	`+ CHECK_EQ(data, hello);`
	`209`	`+ ++static_cast<int>(hint);`
	`210`	`+ },`
	`211`	`+ &callback_calls).ToLocalChecked();`
	`212`	`+ CHECK(buf_obj->IsUint8Array());`
	`213`	`+ ab = buf_obj.As<v8::Uint8Array>()->Buffer();`
	`214`	`+ CHECK_EQ(ab->ByteLength(), sizeof(hello));`
	`215`	`+ }`
	`216`	`+`
	`217`	`+ CHECK_EQ(callback_calls, 1);`
	`218`	`+ CHECK_EQ(ab->ByteLength(), 0);`
	`219`	`+}`

0 commit comments

Comments

0 (0)

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.