n-api: defer Buffer finalizer with SetImmediate

src/env.cc

@@ -641,6 +641,11 @@ void Environment::RunAndClearNativeImmediates() {
           if (!try_catch.HasTerminated())
             FatalException(isolate(), try_catch);
 
+          // We are done with the current callback. Increase the counter so that
+          // the steps below make everything *after* the current item part of
+          // the new list.
+          it++;
+
           // Bail out, remove the already executed callbacks from list
           // and set up a new TryCatch for the other pending callbacks.
           std::move_backward(it, list.end(), list.begin() + (list.end() - it));

src/node_api.cc

@@ -32,21 +32,30 @@ namespace v8impl {
 
 namespace {
 
-class BufferFinalizer: private Finalizer {
+class BufferFinalizer : private Finalizer {
  public:
   // node::Buffer::FreeCallback
   static void FinalizeBufferCallback(char* data, void* hint) {
     BufferFinalizer* finalizer = static_cast<BufferFinalizer*>(hint);
-    if (finalizer->_finalize_callback != nullptr) {
-      NapiCallIntoModuleThrow(finalizer->_env, [&]() {
-        finalizer->_finalize_callback(
-          finalizer->_env,
-          data,
-          finalizer->_finalize_hint);
-      });
-    }
+    finalizer->_finalize_data = data;
+    static_cast<node_napi_env>(finalizer->_env)->node_env()
+        ->SetImmediate([](node::Environment* env, void* hint) {
+      BufferFinalizer* finalizer = static_cast<BufferFinalizer*>(hint);
+
+      if (finalizer->_finalize_callback != nullptr) {
+        v8::HandleScope handle_scope(finalizer->_env->isolate);
+        v8::Context::Scope context_scope(finalizer->_env->context());
+
+        NapiCallIntoModuleThrow(finalizer->_env, [&]() {
+          finalizer->_finalize_callback(
+              finalizer->_env,
+              finalizer->_finalize_data,
+              finalizer->_finalize_hint);
+        });
+      }
 
-    Delete(finalizer);
+      Delete(finalizer);
+    }, hint);
   }
 };
 

test/node-api/test_buffer/test.js

@@ -4,18 +4,25 @@
 const common = require('../../common');
 const binding = require(`./build/${common.buildType}/test_buffer`);
 const assert = require('assert');
+const setImmediatePromise = require('util').promisify(setImmediate);
 
-assert.strictEqual(binding.newBuffer().toString(), binding.theText);
-assert.strictEqual(binding.newExternalBuffer().toString(), binding.theText);
-console.log('gc1');
-global.gc();
-assert.strictEqual(binding.getDeleterCallCount(), 1);
-assert.strictEqual(binding.copyBuffer().toString(), binding.theText);
+(async function() {
+  assert.strictEqual(binding.newBuffer().toString(), binding.theText);
+  assert.strictEqual(binding.newExternalBuffer().toString(), binding.theText);
+  console.log('gc1');
+  global.gc();
+  assert.strictEqual(binding.getDeleterCallCount(), 0);
+  await setImmediatePromise();
+  assert.strictEqual(binding.getDeleterCallCount(), 1);
+  assert.strictEqual(binding.copyBuffer().toString(), binding.theText);
 
-let buffer = binding.staticBuffer();
-assert.strictEqual(binding.bufferHasInstance(buffer), true);
-assert.strictEqual(binding.bufferInfo(buffer), true);
-buffer = null;
-global.gc();
-console.log('gc2');
-assert.strictEqual(binding.getDeleterCallCount(), 2);
+  let buffer = binding.staticBuffer();
+  assert.strictEqual(binding.bufferHasInstance(buffer), true);
+  assert.strictEqual(binding.bufferInfo(buffer), true);
+  buffer = null;
+  global.gc();
+  assert.strictEqual(binding.getDeleterCallCount(), 1);
+  await setImmediatePromise();
+  console.log('gc2');
+  assert.strictEqual(binding.getDeleterCallCount(), 2);
+})().then(common.mustCall());

test/node-api/test_exception/test.js

@@ -9,7 +9,10 @@ const test_exception = require(`./build/${common.buildType}/test_exception`);
 function testFinalize(binding) {
   let x = test_exception[binding]();
   x = null;
-  assert.throws(() => { global.gc(); }, /Error during Finalize/);
+  global.gc();
+  process.on('uncaughtException', (err) => {
+    assert.strictEqual(err.message, 'Error during Finalize');
+  });
 
   // To assuage the linter's concerns.
   (function() {})(x);