xdenser / node-firebird-libfbclient

Firebird SQL binding
MIT License
82 stars 34 forks source link

Patched to be compatible with nan v2.0.9 #59

Closed jacobalberty closed 7 years ago

jacobalberty commented 9 years ago

Nan v2.0 or newer is needed to work with the new Node v4.0. This PR includes patches to work with Nan v2.0.9 and is confirmed to work with v4.0, should probably be tested with v0.12 as well before merge

Edit: This pull request fixes #58

xdenser commented 8 years ago

Compiles but tests do not pass for me under ubuntu 15.04, server on windows. I'll try to look at it next week.

agebrock commented 8 years ago

Getting a lot of those: * set a breakpoint in malloc_error_break to debug node(16520,0x7fff772f4300) malloc: * error for object 0x102831928: pointer being freed was not allocated

jacobalberty commented 8 years ago

Yeah I did some investigation with valgrind and it looks like pointers are being changed, I've been pouring over the documentation for nan to try and understand what's going on, its slow. From what I can tell something is changing the pointers and when the garbage collector hits it uses those modified pointers, which makes no sense to me at all. On Sep 28, 2015 5:52 PM, "Christoph Hagenbrock" notifications@github.com wrote:

Getting a lot of those: * set a breakpoint in malloc_error_break to debug node(16520,0x7fff772f4300) malloc: * error for object 0x102831928: pointer being freed was not allocated

— Reply to this email directly or view it on GitHub https://github.com/xdenser/node-firebird-libfbclient/pull/59#issuecomment-143895782 .

jacobalberty commented 8 years ago

That should be it, all tests pass up to events and for me they still trigger multiple events no matter what version of node-firebird-libfbclient and node combination i use.

jacobalberty commented 8 years ago

Just figured i'd note none of the breaking changes in node 5.0 seem to break these patches so equally compatible with node 4.2 and 5.0

ingumsky commented 7 years ago

The pull request is still not merged with the main repo but the changes jacobalberty suggested work like a charm for me on a number of projects. xdenser could you please merge it?

ingumsky commented 7 years ago

Thank you!

mariuz commented 7 years ago

@xdenser could you also push to npm a new version , thank you 👍