Refactor - use `rvagg/nan` v1.1.0

[TooTallNate]

So, this seems to be pretty close, but not quite there. I'm still getting 2 build errors on my machine.

I'm having trouble specifically with figuring out how to implement the writeObject() and and readObject() functions using nan API.

If one of the nan gurus could take a look that would be very much appreciated :beers: /cc @rvagg @kkoopa @bnoordhuis

[bnoordhuis]

I'm still getting 2 build errors on my machine.

What build errors are those?

I'm having trouble specifically with figuring out how to implement the writeObject() and and readObject() functions using nan API.

I don't know what those methods do, I've never used ref.

[TooTallNate]

What build errors are those?

Well I seemed to fix one of them with a4dd2b0, but now I'm still getting:

  CXX(target) Release/obj.target/binding/src/binding.o
../src/binding.cc:149:1: error: call to function '_Nan_Weak_Callback_write_object_cb' that is neither visible in the template definition nor found by argument-dependent lookup
NAN_WEAK_CALLBACK(write_object_cb) {
^
../node_modules/nan/nan.h:1404:9: note: expanded from macro 'NAN_WEAK_CALLBACK'
        _Nan_Weak_Callback_ ## name(wcbd);                                     \
        ^
<scratch space>:87:1: note: expanded from here
_Nan_Weak_Callback_write_object_cb
^
../src/binding.cc:186:50: note: in instantiation of function template specialization '<anonymous namespace>::write_object_cb<v8::Object, void>' requested here
    NanMakeWeakPersistent(NanNew(p), user_data, &write_object_cb<Object, void>);
                                                 ^
../src/binding.cc:149:1: note: '_Nan_Weak_Callback_write_object_cb' should be declared prior to the call site or in an associated namespace of one of its arguments
NAN_WEAK_CALLBACK(write_object_cb) {
^
../node_modules/nan/nan.h:1409:28: note: expanded from macro 'NAN_WEAK_CALLBACK'
    static NAN_INLINE void _Nan_Weak_Callback_ ## name(                        \
                           ^
<scratch space>:88:1: note: expanded from here
_Nan_Weak_Callback_write_object_cb
^
1 error generated.
make: *** [Release/obj.target/binding/src/binding.o] Error 1

Related to NAN_WEAK_CALLBACK usage... This is on OS X btw, I noticed that on Linux (Travis at least) the compile works, but then the tests segfault :(

I don't know what those methods do, I've never used ref.

The source for them is pretty self-explanatory. They're just static functions that operate on Buffer instances. The idea is that writeObject() would write a Persistent<Object> * to the Buffer space, and readObject() would read that JS object back out (convert the Persistent to a Local basically).

This was possible using V8's raw API in v0.10 and before, I'm just hoping that it's still possible with the newer APIs / nan.

[kkoopa]

I don't see anything immediately wrong at the lines in question. What compiler are you using?

[TooTallNate]

Default OS X compiler that ships with the command line tools (LLVM):

$ g++ --version
Configured with: --prefix=/Applications/Xcode.app/Contents/Developer/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)
Target: x86_64-apple-darwin13.1.0
Thread model: posix

[TooTallNate]

The build seems to be failing on Linux with node v0.11.x as well (https://travis-ci.org/TooTallNate/ref/jobs/26013040)

[kkoopa]

Maybe LLVM sucks? (Or it actually is my code.) Anyway, it doesn't seem to like using the inner callback before it has been declared. Does it work if you add a line declaring the inner callback function to the beginning of the NAN_WEAK_CALLBACK macro?

Something like this:

# define NAN_WEAK_CALLBACK(name) \
template<typename T, typename P> \
static NAN_INLINE void _Nan_Weak_Callback_ ## name( \
const _NanWeakCallbackData<T, P> &data); \
template<typename T, typename P> \
static void name( \
const v8::WeakCallbackData<T, _NanWeakCallbackInfo<T, P> > &data) { \
_NanWeakCallbackData<T, P> wcbd( \
data.GetParameter()); \
_Nan_Weak_Callback_ ## name(wcbd); \
if (wcbd.IsNearDeath()) delete wcbd.GetCallbackInfo(); \
} \
\
template<typename T, typename P> \
static NAN_INLINE void _Nan_Weak_Callback_ ## name( \
const _NanWeakCallbackData<T, P> &data)

[kkoopa]

It's LLVM's fault (although they claim it's mine). http://clang.llvm.org/compatibility.html Unqualified lookup in templates.

[TooTallNate]

@kkoopa Getting this now (after fixing the naming error in your patch above s/v8::WeakCallbackData/_NanWeakCallbackData/... not sure if that's correct):

  CXX(target) Release/obj.target/binding/src/binding.o
../src/binding.cc:186:5: error: no matching function for call to 'NanMakeWeakPersistent'
    NanMakeWeakPersistent(NanNew(p), user_data, &write_object_cb<Object, void>);
    ^~~~~~~~~~~~~~~~~~~~~
../node_modules/nan/nan.h:1416:42: note: candidate function [with T = v8::Object, P = void] not viable: no overload of 'write_object_cb' matching 'typename _NanWeakCallbackInfo<Object, void>::Callback'
      (aka 'void (*)(v8::Persistent<v8::Value>, void *)') for 3rd argument
  NAN_INLINE _NanWeakCallbackInfo<T, P>* NanMakeWeakPersistent(
                                         ^
1 error generated.
make: *** [Release/obj.target/binding/src/binding.o] Error 1

[kkoopa]

There was no such error, I pasted code for the 0.11 version. There's another one for 0.10, look it up in nan.h and do the declaration addition there.

[TooTallNate]

@kkoopa Your patch seems to get things to compile at least. Let's get that upstreamed :)

[kkoopa]

Good. Gotta check that it does not break anything else.

[kkoopa]

L178 won't work on 0.11, Persistent handles cannot be assigned. L186 probably does not work due to p being Persistent, NanMakeWeakPersistent does not take Persistenthandles, because they cannot be reassigned.

[TooTallNate]

@kkoopa Can you check out 77a1c01? At this point everything compiles for me on v0.8, v0.10, and v0.11, which is a good milestone.

However, the tests still segfault for me on all versions, but that's a new issue :p

[TooTallNate]

Fwiw (v0.8.x):

(lldb) bt all
* thread #1: tid = 0x2ab700, 0x000000010018b97a node`v8::internal::GlobalHandles::Destroy(v8::internal::Object**) + 90, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=EXC_I386_GPFLT)
  * frame #0: 0x000000010018b97a node`v8::internal::GlobalHandles::Destroy(v8::internal::Object**) + 90
    frame #1: 0x00000001009e064c binding.node`(anonymous namespace)::WriteObject(v8::Arguments const&) [inlined] v8::Persistent<v8::Object>::Dispose() + 349 at v8.h:4067
    frame #2: 0x00000001009e063e binding.node`(anonymous namespace)::WriteObject(v8::Arguments const&) [inlined] void NanAssignPersistent<v8::Object>(v8::Persistent<v8::Object>&, v8::Handle<v8::Object>) at nan.h:1347
    frame #3: 0x00000001009e063e binding.node`(anonymous namespace)::WriteObject(args=<unavailable>) + 335 at binding.cc:187
    frame #4: 0x000000010014343c node`v8::internal::Builtin_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) + 460
    frame #5: 0x0000234978706362

  thread #2: tid = 0x2ab70f, 0x00007fff8fc3ca3a libsystem_kernel.dylib`__semwait_signal + 10, name = 'SamplerThread'
    frame #0: 0x00007fff8fc3ca3a libsystem_kernel.dylib`__semwait_signal + 10
    frame #1: 0x00007fff8ccbcdc0 libsystem_c.dylib`nanosleep + 200
    frame #2: 0x00007fff8ccbccb2 libsystem_c.dylib`usleep + 54
    frame #3: 0x000000010038a06d node`v8::internal::SamplerThread::Run() + 125
    frame #4: 0x00000001003897fc node`v8::internal::ThreadEntry(void*) + 60
    frame #5: 0x00007fff903c6899 libsystem_pthread.dylib`_pthread_body + 138
    frame #6: 0x00007fff903c672a libsystem_pthread.dylib`_pthread_start + 137

[kkoopa]

L187 probably makes it so the weak callback never gets called, as there will always be a non-weak reference alive. Can't you make the pointer point to the persistent in _NanWeakCallbackInfo instead? The segfaulting is probably due to referencing an object that has been garbage collected.

[TooTallNate]

@kkoopa Boom! You're awesome! d225cf130447bcc693d91f08fc2c50cfe0ae7da7 does the trick, and all tests pass on v0.8, v0.10 and v0.11 for me now! Thanks a bunch dude, I owe you one :)