Calling Javascript function from a C++ callback in

Question

I'm trying to call a registered JS function when a c++ callback is called, but I'm getting a segfault for what I assume is a scoping issue.

 Handle<Value> addEventListener( const Arguments& args ) {
    HandleScope scope;
    if (!args[0]->IsFunction()) {
        return ThrowException(Exception::TypeError(String::New("Wrong arguments")));
    }

    Persistent<Function> fn = Persistent<Function>::New(Handle<Function>::Cast(args[0]));
    Local<Number> num = Number::New(registerListener(&callback, &fn));
    scope.Close(num);
}

When an event happens, the following method is called. I'm assuming that this probably happens on another thread to which V8 is executing JS.

void callback(int event, void* context ) {
    HandleScope scope;
    Local<Value> args[] = { Local<Value>::New(Number::New(event)) };
    Persistent<Function> *func = static_cast<Persistent<Function> *>(context);
    (* func)->Call((* func), 1, args);

    scope.Close(Undefined());
}

This causes a Segmentation fault: 11. Note that if I call the callback function directly with a reference to Persistent from addEventListener(), it executes the function correctly.

I'm assuming that I need a Locker or Isolate? It also looks like libuv's uv_queue_work() might be able to solve this, but since I don't start the thread, I can't see how you would use it.

Answer 1

The problem is that in addEventListener, Persistent<Function> fn is allocated on the stack, and then you're taking the pointer to that to use as a context for the callback.

But, because fn is allocated on the stack, it disappears when addEventListener exits. So withing the callback context now point to some bogus value.

You should allocate some heap space, and put all the data you need in callback there.

Answer 2

I know this question is a bit old, but there has been a pretty major update in nodejs v0.10 to v0.12. V8 changed the behavior of v8::Persistent. v8::Persistent no longer inherits from v8::Handle. I was updating some code and found that the following worked...

  void resize(const v8::FunctionCallbackInfo<Value> &args) {
    Isolate *isolate = Isolate::GetCurrent();
    HandleScope scope(isolate);
    Persistent<Function> callback;
    callback.Reset(isolate, args[0].As<Function>())
    const unsigned argc = 2;
    Local<Value> argv[argc] = { Null(isolate), String::NewFromUtf8(isolate, "success") };
    Local<Function>::New(isolate, work->callback)->Call(isolate->GetCurrentContext()->Global(), argc, argv);
    callback.Reset();
  }

I believe the goal of this update was to make it harder to expose memory leaks. In node v0.10, you would have done something like the following...

  v8::Local<v8::Value> value = /* ... */;
  v8::Persistent<v8::Value> persistent = v8::Persistent<v8::Value>::New(value);
  // ...
  v8::Local<v8::Value> value_again = *persistent;
  // ...
  persistent.Dispose();
  persistent.Clear();

Answer 3

When you declare Persistent<Function> fn in your code, fn is a stack-allocated variable.

fn is a Persistent<Function>, which is a handle class, and it will contain a pointer to some heap-allocated value of type Function, but fn itself is on the stack.

This means that when you call registerListener(&callback, &fn), &fn is taking the address of the handle (type Persistent<Function>), not the address of the Function on the heap. When your function exits, the handle will be destroyed but the Function itself will remain on the heap.

So as a fix, I suggest passing the address of the Function instead of the address of the handle, like this:

Persistent<Function> fn = Persistent<Function>::New(Handle<Function>::Cast(args[0]));
Local<Number> num = Number::New(registerListener(&callback, *fn));

(note that operator* on a Persistent<T> returns a T* rather than the more conventional T&, c.f. http://bespin.cz/~ondras/html/classv8_1_1Handle.html)

You'll also have to adjust callback to account for the fact that context is now a raw pointer to a Function, like this:

Persistent<Function> func = static_cast<Function*>(context);
func->Call((* func), 1, args);

Creating a Persistent<Function> from a raw Function pointer here is OK because we know that context is actually a persistent object.

I've also changed (*func)->Call(...) to func->Call(...) for brevity; they do the same thing for V8 handles.