Do not reset globals when set as returnvalues

[kkoopa]

No description provided.

[brodybits]

Is there a test scenario that can reproduce the bug and verify that it is fixed? I am happy to contribute the test scenario if you like.

[kkoopa]

I don't think there is one, but I don't know how to test this either. The thing is that Globals are supposed to be reset automatically when they go out of scope and their destructors are called.
So, in this code

NAN_METHOD(foo) {
    info.GetReturnValue().Set(Nan::Global<v8::Value>(Nan::New(4)));
}

The Global should be automatically reset as soon as the return value has been set, thereby not leaking. However, the stored return value should not be destroyed by this and remain available until its time has come. Moreover, Globals should support move semantics, so what should really happen is that the underlying storage cell is assigned to the return value container, the Global is cleared so that its storage cell is now only referenced by the return value, then that storage cell should finally be reset when the return value is destroyed.

So it would need to test that there are no memory leaks and no use-after-frees. Also, 78fa1c4 should be included.

[brodybits]

78fa1c4 should be included.

78fa1c4 part of #492 (it took me a few minutes to find it by following the link then changing nodejs to kkoopa in the link).

[brodybits]

To be honest Global is still very new to me. It does make sense to me that this is not so easy to test. I am thinking valgrind is the best way to check for the memory problems such as leaks, double-free(s), and use after free. In addition, we should make sure the code is covered (ideally using automatic code coverage tools).

Hoping to see your fixes land soon.

[kkoopa]

Yeah, holding off on this right now due to the risks of introducing new problems. Will do some manual testing at some point to get these properly integrated.

[kkoopa]

This should go in 2.4