From c882e1c5922a9fb8daab4f07a51c836bb3169046 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Fri, 12 Feb 2021 14:02:02 -0500 Subject: [PATCH 1/3] src: keep reference buffer alive longer Keep a buffer written by WritePointer alive until the finalizers for the buffer to which the pointer has been run have been executed: Refs: https://github.com/node-ffi-napi/ref-napi/issues/54 Signed-off-by: Michael Dawson --- src/binding.cc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/binding.cc b/src/binding.cc index d1fefbd..7d8906e 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -355,6 +355,16 @@ void WritePointer(const CallbackInfo& args) { if (input.IsNull()) { *reinterpret_cast(ptr) = nullptr; } else { + // create a node-api reference and finalizer to ensure that + // the buffer whoes pointer is written can only be + // collected after the finalizers for the buffer + // to which the pointer was written have already run + Reference* ref = new Reference; + *ref = Persistent(args[2]); + args[0].As().AddFinalizer([](Env env, Reference* ref) { + delete ref; + }, ref); + char* input_ptr = GetBufferData(input); *reinterpret_cast(ptr) = input_ptr; } From 4cf8eedb025ca1223c08acdec1f6bb380b1dc2e7 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 11 Mar 2021 11:10:03 -0500 Subject: [PATCH 2/3] limit change to writePointer Update to avoid changing behaviour for _writePointer and limit change to writePointer where objects were already being associated with the buffer. Signed-off-by: Michael Dawson --- lib/ref.js | 2 +- src/binding.cc | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/lib/ref.js b/lib/ref.js index 6d24375..0b55bb5 100644 --- a/lib/ref.js +++ b/lib/ref.js @@ -746,7 +746,7 @@ exports.writeObject = function writeObject (buf, offset, obj) { exports.writePointer = function writePointer (buf, offset, ptr) { debug('writing pointer to buffer', buf, offset, ptr); - exports._writePointer(buf, offset, ptr); + exports._writePointer(buf, offset, ptr, true); exports._attach(buf, ptr); }; diff --git a/src/binding.cc b/src/binding.cc index 7d8906e..abd43ae 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -355,15 +355,17 @@ void WritePointer(const CallbackInfo& args) { if (input.IsNull()) { *reinterpret_cast(ptr) = nullptr; } else { - // create a node-api reference and finalizer to ensure that - // the buffer whoes pointer is written can only be - // collected after the finalizers for the buffer - // to which the pointer was written have already run - Reference* ref = new Reference; - *ref = Persistent(args[2]); - args[0].As().AddFinalizer([](Env env, Reference* ref) { - delete ref; - }, ref); + if ((args.Length() == 4) && (args[3].As() == true)) { + // create a node-api reference and finalizer to ensure that + // the buffer whoes pointer is written can only be + // collected after the finalizers for the buffer + // to which the pointer was written have already run + Reference* ref = new Reference; + *ref = Persistent(args[2]); + args[0].As().AddFinalizer([](Env env, Reference* ref) { + delete ref; + }, ref); + } char* input_ptr = GetBufferData(input); *reinterpret_cast(ptr) = input_ptr; From 63cd2698a50f2b37120524a393af60f079cb7ba0 Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 11 Mar 2021 14:42:07 -0500 Subject: [PATCH 3/3] remove _attach call in writePointer remove _attach call in writePointer as it is no longer needed since a stronger version is done in the native code when true is passed as the fourth parameter --- lib/ref.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/ref.js b/lib/ref.js index 0b55bb5..e457bf9 100644 --- a/lib/ref.js +++ b/lib/ref.js @@ -746,8 +746,12 @@ exports.writeObject = function writeObject (buf, offset, obj) { exports.writePointer = function writePointer (buf, offset, ptr) { debug('writing pointer to buffer', buf, offset, ptr); + // Passing true as a fourth parameter does an a stronger + // version of attach which ensures ptr is only collected after + // the finalizer for buf has run. See + // https://github.com/node-ffi-napi/ref-napi/issues/54 + // for why this is necessary exports._writePointer(buf, offset, ptr, true); - exports._attach(buf, ptr); }; /**