GeezerVison

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

GeezerVison is showing github.com — n-api: keep napi_env alive while it has finalizers · nodejs/node@b69f2dd

File tree

Expand file treeCollapse file tree

Open diff view settings

Filter options

src
- js_native_api_v8.h
- node_api.cc
test/node-api/test_buffer
- test-external-buffer.js

Expand file treeCollapse file tree

Open diff view settings

Collapse file

`‎src/js_native_api_v8.h‎`

Copy file name to clipboardExpand all lines: src/js_native_api_v8.h

+23-5Lines changed: 23 additions & 5 deletions

Original file line number	Diff line number	Diff line change
`@@ -182,26 +182,43 @@ inline v8::Local<v8::Value> V8LocalValueFromJsValue(napi_value v) {`
`182`	`182`
`183`	`183`	`// Adapter for napi_finalize callbacks.`
`184`	`184`	`class Finalizer {`
	`185`	`+ public:`
	`186`	`+ // Some Finalizers are run during shutdown when the napi_env is destroyed,`
	`187`	`+ // and some need to keep an explicit reference to the napi_env because they`
	`188`	`+ // are run independently.`
	`189`	`+ enum EnvReferenceMode {`
	`190`	`+ kNoEnvReference,`
	`191`	`+ kKeepEnvReference`
	`192`	`+ };`
	`193`	`+`
`185`	`194`	`protected:`
`186`	`195`	`Finalizer(napi_env env,`
`187`	`196`	`napi_finalize finalize_callback,`
`188`	`197`	`void* finalize_data,`
`189`		`- void* finalize_hint)`
	`198`	`+ void* finalize_hint,`
	`199`	`+ EnvReferenceMode refmode = kNoEnvReference)`
`190`	`200`	`: _env(env),`
`191`	`201`	`_finalize_callback(finalize_callback),`
`192`	`202`	`_finalize_data(finalize_data),`
`193`		`- _finalize_hint(finalize_hint) {`
	`203`	`+ _finalize_hint(finalize_hint),`
	`204`	`+ _has_env_reference(refmode == kKeepEnvReference) {`
	`205`	`+ if (_has_env_reference)`
	`206`	`+ _env->Ref();`
`194`	`207`	`}`
`195`	`208`
`196`		`- ~Finalizer() = default;`
	`209`	`+ ~Finalizer() {`
	`210`	`+ if (_has_env_reference)`
	`211`	`+ _env->Unref();`
	`212`	`+ }`
`197`	`213`
`198`	`214`	`public:`
`199`	`215`	`static Finalizer* New(napi_env env,`
`200`	`216`	`napi_finalize finalize_callback = nullptr,`
`201`	`217`	`void* finalize_data = nullptr,`
`202`		`- void* finalize_hint = nullptr) {`
	`218`	`+ void* finalize_hint = nullptr,`
	`219`	`+ EnvReferenceMode refmode = kNoEnvReference) {`
`203`	`220`	`return new Finalizer(`
`204`		`- env, finalize_callback, finalize_data, finalize_hint);`
	`221`	`+ env, finalize_callback, finalize_data, finalize_hint, refmode);`
`205`	`222`	`}`
`206`	`223`
`207`	`224`	`static void Delete(Finalizer* finalizer) {`
`@@ -214,6 +231,7 @@ class Finalizer {`
`214`	`231`	`void* _finalize_data;`
`215`	`232`	`void* _finalize_hint;`
`216`	`233`	`bool _finalize_ran = false;`
	`234`	`+ bool _has_env_reference = false;`
`217`	`235`	`};`
`218`	`236`
`219`	`237`	`class TryCatch : public v8::TryCatch {`

Collapse file

`‎src/node_api.cc‎`

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

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

Original file line number	Diff line number	Diff line change
`@@ -724,7 +724,8 @@ napi_status napi_create_external_buffer(napi_env env,`
`724`	`724`
`725`	`725`	`// The finalizer object will delete itself after invoking the callback.`
`726`	`726`	`v8impl::Finalizer* finalizer = v8impl::Finalizer::New(`
`727`		`- env, finalize_cb, nullptr, finalize_hint);`
	`727`	`+ env, finalize_cb, nullptr, finalize_hint,`
	`728`	`+ v8impl::Finalizer::kKeepEnvReference);`
`728`	`729`
`729`	`730`	`auto maybe = node::Buffer::New(isolate,`
`730`	`731`	`static_cast<char*>(data),`

Collapse file

`‎test/node-api/test_buffer/test-external-buffer.js‎`

Copy file name to clipboard

+14Lines changed: 14 additions & 0 deletions

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,14 @@`
	`1`	`+'use strict';`
	`2`	`+`
	`3`	`+const common = require('../../common');`
	`4`	+const binding = require(`./build/${common.buildType}/test_buffer`);
	`5`	`+const assert = require('assert');`
	`6`	`+`
	`7`	`+// Regression test for https://github.com/nodejs/node/issues/31134`
	`8`	`+// Buffers with finalizers are allowed to remain alive until`
	`9`	`+// Environment cleanup without crashing the process.`
	`10`	+// The test stores the buffer on `process` to make sure it lives as long as
	`11`	`+// the Context does.`
	`12`	`+`
	`13`	`+process.externalBuffer = binding.newExternalBuffer();`
	`14`	`+assert.strictEqual(process.externalBuffer.toString(), binding.theText);`

0 commit comments

Comments

0 (0)

You are visitor 01402520 — 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.