Node addon fail using PrototypeTemplate :(

[Laurie Harper]

Trimming my code down to the minimum in order to post, I found the problem but don't understand *why* it was a problem :-) Can anyone explain? It's working fine now I've changed Initialize() to use ->InstanceTemplate() instead of ->PrototypeTemplate(); why does the latter not work?

Code follows:

Persistent<FunctionTemplate> Vector::ctor;

void
Vector::Initialize(Handle<Object> target) {
HandleScope scope;

ctor = Persistent<FunctionTemplate>::New(FunctionTemplate::New(New));
ctor->InstanceTemplate()->SetInternalFieldCount(1);
ctor->SetClassName(String::NewSymbol("Vector"));

// ctor->PrototypeTemplate()->SetAccessor(String::NewSymbol("x"), GetX, SetX);
ctor->InstanceTemplate()->SetAccessor(String::NewSymbol("x"), GetX, SetX);

target->Set(String::NewSymbol("Vector"), ctor->GetFunction());
}

Handle<Value>
Vector::GetX(Local<String> property, const AccessorInfo& info) {
HandleScope scope;
// Vector* v = ObjectWrap::Unwrap<Vector>(info.This()); // This vs. Holder?
Vector* v = ObjectWrap::Unwrap<Vector>(info.Holder()); // This vs. Holder?
Local<Number> result = Number::New(v->m_btVector3->getX());
return scope.Close(result);
}

void
Vector::SetX(Local<String> property, Local<Value> value, const AccessorInfo& info) {
Vector* v = ObjectWrap::Unwrap<Vector>(info.Holder());
v->m_btVector3->setX(value->NumberValue());
}

Handle<Value>
Vector::GetX(Local<String> property, const AccessorInfo& info) {
HandleScope scope;
// Vector* v = ObjectWrap::Unwrap<Vector>(info.This()); // This vs. Holder?
Vector* v = ObjectWrap::Unwrap<Vector>(info.Holder()); // This vs. Holder?
Local<Number> result = Number::New(v->m_btVector3->getX());
return scope.Close(result);
}

void
Vector::SetX(Local<String> property, Local<Value> value, const AccessorInfo& info) {
Vector* v = ObjectWrap::Unwrap<Vector>(info.Holder());
v->m_btVector3->setX(value->NumberValue());
}

--
Laurie Harper
http://laurie.holoweb.net/

[Laurie Harper]

Oops, the implementation of New() got left out... here it is:

Handle<Value>
Vector::New(const Arguments &args) {
float x, y, z;
HandleScope scope;

if (args.Length() == 1 && args[0]->IsArray()) {
x = args[0]->ToObject()->Get(0)->NumberValue();
y = args[0]->ToObject()->Get(1)->NumberValue();
z = args[0]->ToObject()->Get(2)->NumberValue();
} else {
x = args[0]->IsNumber() ? args[0]->NumberValue() : 0;
y = args[1]->IsNumber() ? args[1]->NumberValue() : 0;
z = args[2]->IsNumber() ? args[2]->NumberValue() : 0;
}

Vector* obj = new Vector(x, y, z);
obj->Wrap(args.This());
return args.This();
}

> --
> You received this message because you are subscribed to the Google Groups "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com.
> To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.

[Laurie Harper]

Follow-up question: more woes involving prototypes on objects defined in c++. Same code as before, with this in JavaScript:

var Vector = require(...);
function f() {}; f.prototype = Vector.prototype;
function g() { Vector.apply(this, arguments) }
var v = new g(1,2,3);

Results in an assertion error:

Assertion failed: (handle->InternalFieldCount() > 0), function Wrap, file /Users/laurie/.nvm/v0.4.5/include/node/node_object_wrap.h, line 61.

Best as I can tell, from digging through various Node and v8 list discussions, it's because args.This() returns an instance of 'g', not Vector; however, using args.Holder() doesn't seem to make any difference. I'm not sure what else to try changing...

On 2011-05-18, at 7:28 PM, Laurie Harper wrote:

> --
> You received this message because you are subscribed to the Google Groups "nodejs" group.
> To post to this group, send email to nod...@googlegroups.com.
> To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.
>

--
Laurie Harper
http://laurie.holoweb.net/

[Marco Rogers]

Laurie:

It sounds like you're doing cool stuff with addons. Is your project up on github?

The trouble you're running into is curious. But it's also never really clear how the v8 APIs are meant to be used. I would think your original example using PrototypeTemplate would work as expected. This is all still new territory. But as I understand it, when you use InstanceTemplate, it's the same as this:

function Vector() {

   this.x = function() { ... }

}

Each object created from this constructor would have a copy of the property you set. But using PrototypeTemplate should be the equivalent of this:

function Vector() {

}

Vector.prototype.x = function() { ... }

So there is only 1 version of "x". I'm sure that's what you were going for. I've never tried it, but I would assume the same thing. You should ask about that on the v8-users list. Post your cross link here, because I'm also interested.

As for the assertion failure, that's tougher. It involved node's ObjectWrap functionality. And there is also the fact that you are using apply() to call Vector in an odd context. Here's my limited understanding, without actually running any tests. When you create a FunctionTemplate based on your C++ New function, you can call it as a regular javascript function. But when you call it as a constructor with "new", a new object gets created to be the context. That object is created from the InstanceTemplate that you set up in your Initialize function. Or at least it's supposed to be. But instead you're passing in the context object from g(). So when Vector is called, "this" is not the object that New expects it to be. Does that make sense?  And this leads directly to the assertion failure, because on your InstanceTemplate in Initialize you set the InternalFieldCount. But on the object from g() there is none. So when ObjectWrap tries to do it's thing, it finds that the handle it received is not in the proper state.

Whew. So how to fix this? I'm not sure. I would like to know more about what you're trying to achieve and why you're doing it this way with wrapping Vector in other constructors. I hope my explanations help at least. Maybe some v8 folks will chime in here and provide better background.

:Marco

[Laurie Harper]

On 2011-05-19, at 1:46 AM, Marco Rogers wrote:
> It sounds like you're doing cool stuff with addons. Is your project up on github?

Meh, it's not even at a point worth checking into a local repo yet, let alone pushing ;-)

> The trouble you're running into is curious. But it's also never really clear how the v8 APIs are meant to be used. I would think your original example using PrototypeTemplate would work as expected. This is all still new territory. But as I understand it, [...]

>
> So there is only 1 version of "x". I'm sure that's what you were going for. I've never tried it, but I

Yes, that's exactly what I was assuming. However, the properties set on the prototype don't show up on the constructed object in JavaScript, and accessing them crashes Node (a similar assertion error to what I posted subsequently). My only theory at the moment is that accessors set on the prototype aren't allowed to depend on instance data, though you would expect that to work as it does in JavaScript...

> would assume the same thing. You should ask about that on the v8-users list. Post your cross link here, because I'm also interested.

I'll do that and add a link to the thread here.

> As for the assertion failure, that's tougher. It involved node's ObjectWrap functionality. And there is also the fact that you are using apply() to call Vector in an odd context. Here's my

I don't see what's odd about it; that's a standard pattern for applying inheritance in JavaScript -- a dummy function (f) to avoid calling the base type constructor while setting up the prototype chain, then an explicit call to the base constructor in the sub-type constructor, which is the apply().

> limited understanding, without actually running any tests. When you create a FunctionTemplate based on your C++ New function, you can call it as a regular javascript function. But when you call it as a constructor with "new", a new object gets created to be the context. That object is created from the InstanceTemplate that you set up in your Initialize function. Or at least it's supposed to be. But instead you're passing in the context object from g(). So when Vector is called, "this" is not the object that New expects it to be. Does that make sense? And this leads directly to the assertion failure, because on your InstanceTemplate in Initialize you set the InternalFieldCount. But on the object from g() there is none. So when ObjectWrap tries to do it's thing, it finds that the handle it received is not in the proper state.

Yes, that makes sense; the object the JavaScript runtime creates when using 'new' is just a plain object; JS doesn't look at the prototype chain at that stage to figure out what type of object to create as the instance. Effectively, this inheritance pattern relies on the constructor functions themselves being generic, which I guess doesn't work with C++ proxies...

> Whew. So how to fix this? I'm not sure. I would like to know more about what you're trying to achieve and why you're doing it this way with wrapping Vector in other constructors. I hope my explanations help at least. Maybe some v8 folks will chime in here and provide better background.

I'm doing it this way because as far as I know that's how it's meant to be done... if there's a different pattern I could apply I'd love to try it :-) However, my understanding is that to create a binding to an existing C++ API for Node, you need to create a proxy extending ObjectWrap to represent objects from the native API to the JavaScript runtime.

I've written a suite of JavaScript unit tests that use the C++ proxied object as if it were a normal JavaScript object, checking that everything works as expected, because I'm pretty fuzzy on how some of the C++ side is meant to be done -- and these problems show I don't have it quite right yet ;-)

Thanks for the responses; it looks like I'll need to follow up on the v8 list to dig deeper...

[Marco Rogers]

> The trouble you're running into is curious. But it's also never really clear how the v8 APIs are meant to be used. I would think your original example using PrototypeTemplate would work as expected. This is all still new territory. But as I understand it, [...]
>
> So there is only 1 version of "x". I'm sure that's what you were going for. I've never tried it, but I

Yes, that's exactly what I was assuming. However, the properties set on the prototype don't show up on the constructed object in JavaScript, and accessing them crashes Node (a similar assertion error to what I posted subsequently). My only theory at the moment is that accessors set on the prototype aren't allowed to depend on instance data, though you would expect that to work as it does in JavaScript...

Have you tried it with a regular function rather than an accessor? Any difference?

> As for the assertion failure, that's tougher. It involved node's ObjectWrap functionality. And there is also the fact that you are using apply() to call Vector in an odd context. Here's my

I don't see what's odd about it; that's a standard pattern for applying inheritance in JavaScript -- a dummy function (f) to avoid calling the base type constructor while setting up the prototype chain, then an explicit call to the base constructor in the sub-type constructor, which is the apply().

By "odd" I only meant that you're not using the InstanceTemplate that you actually set up for the New function. Just means you're circumventing your own preparation.

 

Yes, that makes sense; the object the JavaScript runtime creates when using 'new' is just a plain object; JS doesn't look at the prototype chain at that stage to figure out what type of object to create as the instance. Effectively, this inheritance pattern relies on the constructor functions themselves being generic, which I guess doesn't work with C++ proxies...

I'm not sure it's a lost cause. Search on the v8 list for args.Holder() as opposed to args.This(). It's a different way to access the context of a function call, and it has some other properties. Here's the couple of reference links I have.

https://groups.google.com/d/topic/v8-users/Axf4hF_RfZo/discussion

https://groups.google.com/d/topic/v8-users/fK9PBWxJxtQ/discussion

I'm doing it this way because as far as I know that's how it's meant to be done... if there's a different pattern I could apply I'd love to try it :-) However, my understanding is that to create a binding to an existing C++ API for Node, you need to create a proxy extending ObjectWrap to represent objects from the native API to the JavaScript runtime.

I'm wondering less about the binding and more about your sub-classing code. Are you missing the step of creating a new object for the prototype of g()?

var Vector = require(...);
function f() {};  f.prototype = Vector.prototype;
function g() { Vector.apply(this, arguments) }

// added this line

g.prototype = new f();

var v = new g(1,2,3);

This is the part that actually puts Vector in your prototype chain. Without it, all you're doing is running the Vector constructor on your g context.  You might also think about using util.inherits from Node which is pretty simple and uses the Object.create pattern. I'd be interested to see how that plays with constructors from bindings.

https://github.com/joyent/node/blob/master/lib/util.js#L423-428

FYI, I'm interested in this line of experimentation because I have the same problem. The DOM objects that are created by libxmljs also don't behave like normal js objects in these cases. It has been on my list to revamp things once I get some time and more info about the issues. If you share some code with me, I'd be happy to run some tests and help you try to work through things.

:Marco

[Laurie Harper]

Sorry, forgot to post the link to the question on the v8 list:

http://groups.google.com/group/v8-users/browse_thread/thread/3d473ed223a4139f

Responses inline:

On 2011-05-19, at 6:42 PM, Marco Rogers wrote:

> The trouble you're running into is curious. But it's also never really clear how the v8 APIs are meant to be used. I would think your original example using PrototypeTemplate would work as expected. This is all still new territory. But as I understand it, [...]
>
> So there is only 1 version of "x". I'm sure that's what you were going for. I've never tried it, but I

Yes, that's exactly what I was assuming. However, the properties set on the prototype don't show up on the constructed object in JavaScript, and accessing them crashes Node (a similar assertion error to what I posted subsequently). My only theory at the moment is that accessors set on the prototype aren't allowed to depend on instance data, though you would expect that to work as it does in JavaScript...

Have you tried it with a regular function rather than an accessor? Any difference?

Yes; using Node's NODE_SET_PROTOTYPE_METHOD macro to put a function on the prototype works fine (as far as I've tested so far...).

> As for the assertion failure, that's tougher. It involved node's ObjectWrap functionality. And there is also the fact that you are using apply() to call Vector in an odd context. Here's my

I don't see what's odd about it; that's a standard pattern for applying inheritance in JavaScript -- a dummy function (f) to avoid calling the base type constructor while setting up the prototype chain, then an explicit call to the base constructor in the sub-type constructor, which is the apply().

By "odd" I only meant that you're not using the InstanceTemplate that you actually set up for the New function. Just means you're circumventing your own preparation.

That was my conclusion -- see the v8 thread for my analysis of what 'went wrong'. The New() function *does* get called, but it looks like the way Arguments::This() is bound breaks my implementation; I assume there's a different way of doing things to handle that case correctly.

I'm not sure it's a lost cause. Search on the v8 list for args.Holder() as opposed to args.This(). It's a different way to access the context of a function call, and it has some other properties. Here's the couple of reference links I have.

https://groups.google.com/d/topic/v8-users/Axf4hF_RfZo/discussion

https://groups.google.com/d/topic/v8-users/fK9PBWxJxtQ/discussion

Thanks for those references; the first I'd seen, and didn't seem applicable; the second gives me a couple of ideas to test... ;-)

I'm doing it this way because as far as I know that's how it's meant to be done... if there's a different pattern I could apply I'd love to try it :-) However, my understanding is that to create a binding to an existing C++ API for Node, you need to create a proxy extending ObjectWrap to represent objects from the native API to the JavaScript runtime.

I'm wondering less about the binding and more about your sub-classing code. Are you missing the step of creating a new object for the prototype of g()?

var Vector = require(...);
function f() {};  f.prototype = Vector.prototype;
function g() { Vector.apply(this, arguments) }

// added this line

g.prototype = new f();
var v = new g(1,2,3);

Oops, it was missing in my post but not in my test-suite; sorry for any confusion! (Same goof on the v8 list, too; just posted a correction there.)

This is the part that actually puts Vector in your prototype chain. Without it, all you're doing is running the Vector constructor on your g context.  You might also think about using util.inherits from Node which is pretty simple and uses the Object.create pattern. I'd be interested to see how that plays with constructors from bindings.

https://github.com/joyent/node/blob/master/lib/util.js#L423-428

[Yeah, I should test that too; it's a simpler inheritance pattern.] In fact, added to my test suite: result is the same.

FYI, I'm interested in this line of experimentation because I have the same problem. The DOM objects that are created by libxmljs also don't behave like normal js objects in these cases. It has been on my list to revamp things once I get some time and more info about the issues. If you share some code with me, I'd be happy to run some tests and help you try to work through things.

Happy to :) I'll get my test code wrapped up into a gist (or a separate project on github) to make it easier to experiment and share...

[Laurie Harper]

I put all the code into a gist which can be downloaded, built and tested locally:

$ git clone git://gist.github.com/982157.git

$ cd 982157

$ node-waf configure build 

$ node VectorTests.js 

Vector is function Vector() { [native code] }

Vector.proto is [object Object]

v is { x: 1 }

v.x is  1

v is { x: 9 }

v.x is  9

Assertion failed: (handle->InternalFieldCount() > 0), function Wrap, file /Users/laurie/.nvm/v0.4.5/include/node/node_object_wrap.h, line 61.

Abort trap

L.

--
You received this message because you are subscribed to the Google Groups "nodejs" group.
To post to this group, send email to nod...@googlegroups.com.
To unsubscribe from this group, send email to nodejs+un...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/nodejs?hl=en.

[Marco Rogers]

Cool.  I'll check this out some time tomorrow.

Marco Rogers
marco....@gmail.com

Life is ten percent what happens to you and ninety percent how you respond to it.
- Lou Holtz

[Laurie Harper]

FWIW, I have a possible work-around for the inheritance case, which I added to the gist as a comment:

https://gist.github.com/982157#gistcomment-31654

Doesn't seem like the best solution to me though; it really shouldn't be necessary :(

L.

[Matthias Ernst]

On Fri, May 20, 2011 at 6:40 AM, Laurie Harper <lau...@holoweb.net> wrote:
> FWIW, I have a possible work-around for the inheritance case, which I added
> to the gist as a comment:
> https://gist.github.com/982157#gistcomment-31654
> Doesn't seem like the best solution to me though; it really shouldn't be
> necessary :(
> L.

> function g() { Vector.apply(this, arguments) };

With your workaround, the native Vector constructor potentially
constructs its own instance and returns that. IIUC you need to
"*return* Vector.apply(this, arguments)" from g() in order to replace
the instance that got created by "new" in the first place.

If that doesn't work for you, you'll have to live with with regular JS
objects being passed to your native methods. You won't be able to use
internal fields, but instead, you could use Set/GetHiddenValue which
is probably slower but works on any object. You'd need to fork
ObjectWrap for this, of course.

I'm not positive why your property handler isn't called, but using the
instance template is fine. In my understanding the set of native
property handlers on an object template very much behave as if they
are on invisible nodes in the prototype chain: instance->(invisible
instance_template)->prototype->(invisible prototype_template).

Matthias

[Laurie Harper]

On 2011-05-20, at 2:20 AM, Matthias Ernst wrote:
> On Fri, May 20, 2011 at 6:40 AM, Laurie Harper <lau...@holoweb.net> wrote:
>> FWIW, I have a possible work-around for the inheritance case, which I added
>> to the gist as a comment:
>> https://gist.github.com/982157#gistcomment-31654
>> Doesn't seem like the best solution to me though; it really shouldn't be
>> necessary :(
>> L.
>
>> function g() { Vector.apply(this, arguments) };
>
> With your workaround, the native Vector constructor potentially
> constructs its own instance and returns that. IIUC you need to
> "*return* Vector.apply(this, arguments)" from g() in order to replace
> the instance that got created by "new" in the first place.

That might work, for this particular use case, but wouldn't solve the general case of calling a constructor function without 'new', which happens quite often in general. My goal here is to understand what's happening in the v8 APIs and 'do the right thing' so that proxied objects behave the same as they would if they were native JS objects.

If I absolutely can't find a way to do that, I'll end up using whatever work-arounds are needed in a JS layer that wraps the c++ layer that wraps the native library... but... ugh! ;-)

> If that doesn't work for you, you'll have to live with with regular JS
> objects being passed to your native methods. You won't be able to use
> internal fields, but instead, you could use Set/GetHiddenValue which
> is probably slower but works on any object. You'd need to fork
> ObjectWrap for this, of course.

Not really an option; saying "don't 'npm install' this unless you're using my fork of Node" doesn't really cut it ;-) Though I suppose if I could sub-class ObjectWrap in my addon code I could take that approach.

> I'm not positive why your property handler isn't called, but using the
> instance template is fine. In my understanding the set of native
> property handlers on an object template very much behave as if they
> are on invisible nodes in the prototype chain: instance->(invisible
> instance_template)->prototype->(invisible prototype_template).

That's useful information if it's accurate, I don't need to address the InstanceTemplate vs. PrototypeTemplate problem in this case. Again, though, there's a general case to solve: what if the property is a default whose value isn't overridden in the constructor? In other words, 'Vector.x = 999; v = new Vector(); v.x;' should return 999. As the code is now it would return 0 (which is intended). If I wanted that behaviour, though, I'd need PrototypeTemplate to work.

[Vyacheslav Egorov]

Hi Laurie,

Please take a close look at your code:

function f() {}; f.prototype = Vector.prototype;

function g() { Vector.apply(this, arguments) };
g.prototype = new f();
v = new g(1,2,3);

The question you should ask yourself is: is there any object here that
was actually _created_ with Vector constructor?

The answer is no. There is one object that was passed to Vector
constructor to be _initialized_ by it, but there are none that was
actually created by doing new Vector(...).

Because this object inside g() was not created by Vector constructor
it did not get an internal field. That's why you get an assertion.

--
Vyacheslav Egorov

[Laurie Harper]

Understood; the question is how to code the c++ proxy to work for this usage. Failing that, at this point I'd settle for a way to write a JavaScript wrapper that'll work for this usage (i.e. the JS constructor function gets called this way, and calls the proxy constructor in a way that doesn't break).

Currently I can't do the JS wrapper approach either, since my code doesn't work when methods on the proxy are invoked through the prototype chain of a JS object. That may be as simple as changing args.This() to args.Holder(); I haven't explored that yet, since I wanted to get object construction right first...

L.