Shared_ptr behaviour is different accross Node & Asm.js

[arcanis]

In the following code, insertChild takes a shared_ptr by copy, and getChild returns a shared_ptr by copy:

let a = new Yoga.Node();
let b = new Yoga.Node();

a.insertChild(b, 0);
let c = a.getChild(0);

console.log(b === c);

I get true on Node.js, and false on asm.js. I think that false is the correct behaviour, since it gets harder to know if an object has been released or not if each shared_ptr instance is the same on the JS side. It would mean that if a shared_ptr is used across two parts of the application, then each would need to be aware if the other actually still need the pointer to avoid deleting it accidentally.

(You can check the issue referenced below for more details about why I needed and how I used shared_ptrs - maybe I've missed something)

[jjrv]

There's an undocumented feature (slightly broken in master) for this. It's now fixed in develop and I'll document it. #define NBIND_DUPLICATE_POINTERS 1 should make Node.js work identically to asm.js. Can you test if it works for you?

Rationale for the default behavior:

On Node.js it's possible for the entire JavaScript app to only hold a single shared_ptr, shared across all references made within the JavaScript heap. Only when all the relevant objects get garbage collected, the single reference is released. Since garbage collection is completely transparent and two references to the same object have no logical difference, it makes sense for them to compare as equal.

On asm.js this is less practical due to lack of any garbage collection hooks. Since you need to free things manually, it seems safer to free each reference manually rather than free them all at once, possibly including references still in use elsewhere. This means the references aren't entirely equal, since freeing them has a different outcome (each one needs to be freed separately and freeing the same one again does nothing).

The free method is a problem on Node.js though... Currently it immediately releases the single shared pointer, which differs from asm.js. Maybe actually on Node.js it shouldn't do anything at all and there should be another mechanism for immediately releasing all references held by JavaScript? This way you could always use the same code, but on Node.js freeing is deferred to the next GC and any mistakes made with manual freeing have no effect.

[arcanis]

I've just tried with NBIND_DUPLICATE_POINTERS, and everything looks fine! Many thanks :)

The folks at Yoga had an interesting point regarding the memory management, tho: maybe it would be interesting to have a way to prevent garbage collection altogether, without using shared_ptrs (which need to be eventually manually released with asm.js). Projects would be able to chose which approach is more suited to their interface.

Maybe something like:

function makeNode() {
    var node = new lib.Node();
    lib.preventGarbageCollection(node);
    return node;
}

Which would be a no-op inside browsers.

[jjrv]

If you allocate an object using new from C++ and return a pointer to JavaScript, it'll use a normal pointer (since C++ owns the object). If you call new from JavaScript or return the object by value from C++, it'll use a shared_ptr (since JavaScript owns the object and at least Node.js is capable of managing it properly).

Is there any benefit to not using shared_ptr when JavaScript owns an object?

[arcanis]

I think the main benefit is not having to manually releasing the shared_ptr, and rather trust the user & the library to free the objects when they see it fit. It would be a lower level form of memory management, shared pointers being the higher level with extra safety.

[jjrv]

I suppose that sometimes makes things easier on asm.js but can only be more dangerous on Node.js. It could probably be implemented as a policy parameter added to the NBIND_CLASS macro. That will be a bit complicated though...

For now the easiest way to get that behavior is to wrap the class in JavaScript and instead of calling the C++ constructor, call a static method that calls new in C++ and returns the pointer. Also methods returning such objects by value need to be wrapped to move the object to the C++ heap and return a pointer instead...