NAN 2.0 support (done) #221
Conversation
1.0.3
On Windows we don't need self_thread, so it has moved to Unix only code.
Conflicts: src/callback_info.cc src/ffi.h
|
@TooTallNate Do you know someone in the NAN team who might willing to help us? I think the solution have to be pretty obvious for someone with a good internal knowledge of NAN. If you could summon someone there, I'm gonna lead him to the issue if the above description is too sparse. |
|
It's like V8 all over again 😨 |
Fix errors
|
Ok, that helped, thanks a lot. However 0.10 is still failing, but I'm gonna fix it later today or tomorrow. The main issue was the recent v8 compatibility. |
|
@kkoopa I think there is and issue with the new Nan::NewBuffer on 0.10.40. It looks like its free callback never gets invoked after doing a gc. Here we create a Buffer with a GC callback: There is an unit test, where we nullify the reference of the above Buffer followed by a gc() call: https://github.com/unbornchikken/node-ffi/blob/nan2/test/callback.js#L82 This should invoke that free callback, but it never happens on 0.10.40. 0.12 and io.js is OK. Is this really a NAN issue or am I screwing up something? |
|
Likely fixed in nodejs/nan@bc45db2. Try the memleak branch. |
|
I can confirm that this commit fixes the above Buffer issue. Cool. Now, we gotta wait while this gets released then ffi's NAN 2.0.x support gets completed. Thank you again! |
|
@TooTallNate now ffi's side is completed, you could merge this. But unfortunately install gets failed with io.js 3.0.0 because ref doesn't have NAN 2.0 support implemented. |
|
As of TooTallNate/ref@563af1a ref does have support for NAN 2.0 so I assume this PR can safely be merged, @TooTallNate ? |
|
Refer to the PR's title: Io.js 3.0 support -(blocked, waiting for nodejs/node#2487 gets released) |
|
But I can see that commit has been deployed in io.js 3.2. I'll have to check it later. |
|
I'm closing this. Gonna open up an other PR for this with a corrected commit history. @kkoopa thank you for your help again! |
Squashed commit of the following: commit 9418dd7 Author: unbornchikken <gabor.mezo@outlook.com> Date: Sat Aug 29 21:13:45 2015 +0200 license extended to current year commit 72eed31 Author: unbornchikken <gabor.mezo@outlook.com> Date: Thu Aug 27 09:04:18 2015 +0200 references fixed commit a4b74e3 Author: unbornchikken <gabor.mezo@outlook.com> Date: Thu Aug 6 21:49:33 2015 +0200 done commit 6f024fa Author: unbornchikken <gabor.mezo@outlook.com> Date: Mon Aug 3 12:26:22 2015 +0200 whitespace fixes commit a6d494e Author: unbornchikken <gabor.mezo@outlook.com> Date: Mon Aug 3 10:26:54 2015 +0200 ffi_test.cc wrap pointer fixd commit 01ea563 Author: Benjamin Byholm <bbyholm@abo.fi> Date: Sun Aug 2 15:25:10 2015 +0300 Fix errors commit 99b4983 Author: unbornchikken <gabor.mezo@outlook.com> Date: Thu Aug 27 08:49:14 2015 +0200 Merge master Fix errors Merge pull request #1 from kkoopa/nan2 Fix errors ffi_test.cc wrap pointer fixd whitespace fixes done commit dc0e555 Author: unbornchikken <gabor.mezo@outlook.com> Date: Sat Aug 1 12:48:51 2015 +0200 handle scope crash commit 66f877d Author: unbornchikken <gabor.mezo@outlook.com> Date: Fri Jul 31 21:50:15 2015 +0200 nan2 ok commit 8ae22da Author: unbornchikken <gabor.mezo@outlook.com> Date: Thu Aug 27 09:14:17 2015 +0200 nan2 support impl begin Fixes #219. Refs #221. Closes #231.
This is an in-progress PR. There is an issue that have to be resolved before merge.
When callback queue items gets processed in CallbackInfo::WatcherCallback which is invoked by libuv uv_async_send(&g_async) in callback_info.cc 192, the control flow eventually reaches CallbackInfo::DispatchToV8 at line 71, and there is a Nan::NewBuffer call. But v8 crashes there by raising the following error:
Which is odd because there is a Nan::HandleScope on top of DispatchToV8 definitely (at line 53).
io.js 1.x is not affected, I'm getting this with 2.5. I believe that there is some magic required to make a bare libuv callback v8 GC compatible that is not present in the new Nan::HandleScope.