Nan::Callback::Call improperly garbage collects the return value under Node 10.12, causing segfaults

[murgatroid99]

Under Node 10.12, calling the Call method of a Nan::Callback object returns a garbage object that appears to have been prematurely garbage collected. I have put together a toy extension that demonstrates this:

main.cc:

#include <node.h>
#include <nan.h>

NAN_METHOD(RunCallback) {
  if (!info[0]->IsFunction()) {
    return Nan::ThrowTypeError("Argument must be  function");
  }
  Nan::Callback *callback = new Nan::Callback(v8::Local<v8::Function>::Cast(info[0]));
  int argc = 0;
  v8::Local<v8::Value> argv[argc] = {};
  v8::Local<v8::Value> result = callback->Call(argc, argv);
  if (result->IsNumber()) {
    info.GetReturnValue().Set(Nan::True());
  } else {
    info.GetReturnValue().Set(Nan::False());
  }
}

void init(v8::Local<v8::Object> exports) {
  Nan::Set(exports, Nan::New("runCallback").ToLocalChecked(),
           Nan::GetFunction(Nan::New<v8::FunctionTemplate>(RunCallback)).ToLocalChecked());
}

NODE_MODULE(callback_test, init);

index.js:

const callbackTest = require('./build/Release/callback_test.node');

console.log(callbackTest.runCallback(() => {return 1;}));

binding.gyp:

{
  "targets": [
    {
      "target_name": "callback_test",
      "sources": [ "main.cc" ],
      "include_dirs" : [
        "<!(node -e \"require('nan')\")"
      ]
    }
  ]
}

Running index.js with Node 10.11 correctly outputs "true", and running it with Node 10.12 causes a segmentation fault in result->IsNumber(). The code works correctly under Node 10.12 if the callback->Call line is replaced with

v8::Local<v8::Value> result = Nan::Call(*callback, argc, argv).ToLocalChecked();

[addaleax]

I think the issue is that this line requires an additional scope.Escape()? 1caf258#diff-558c37be49a47656e3ff41806c24bd33R1624 /cc @kkoopa

[kkoopa]

Yes, I am currently waiting for a flight and cannot check it thoroughly, but it appears to be so.

…

On October 17, 2018 4:44:32 PM EDT, Anna Henningsen ***@***.***> wrote: I think the issue is that this line requires an additional `scope.Escape()`? 1caf258#diff-558c37be49a47656e3ff41806c24bd33R1624 /cc @kkoopa

[Flarna]

The change suggested by @addaleax solves the issue.
I was able to reproduce this and created a PR: #817