0.11-wip

[rvagg]

Copied from #89

• Allow saving of empty values. #85 Allow saving of empty values
• Fix leak of options attributes. #88 Fix leak of options attribute
• LevelDB 1.15 #84 Upgrade LevelDB to 0.15.0
• Something related to Update node-leveldown homepage url. #82 - need to document the forks in the README so when you go to their "homepage" you get information that's actually relevant to the forks
• Remove -Wno-unused-local-typedefs from binding.gyp. This is just something I put in there for OCD purposes, the current V8 in master falls foul of this new gcc warning and they still haven't fixed it but we might just put up with it for now since it seems to cause problems on older gcc (CENTOS ARGH!)
• Adopt node-pre-gyp for binary distributables--minly a concern for Windows but will also be helpful for Mac I imagine; not so concerned about Linux and it's actually the most diverse and difficult target
• Upgrade to NAN 0.9 (unreleased)
• Update forks

[rvagg]

I've added node-pre-gyp adoption for the list, it'd be nice if we can get at least Windows pre-builds distributed this way, Mac would also be useful but not as urgent as Windows. I'm not sure if Linux is a practical target because of the diverse nature of the platform (plus every Linux box comes with compilers anyway). We'll see.

Until we have a better solution, node-pre-gyp is the best there is and I'm keen to encourage work on that front. It's not blessed by core, it doesn't have a lot of nice features that people keep on saying are necessary (like signing) but at least someone took the first step, so w00t!

[rvagg]

and I should say, I'm dubious about Windows support here atm, [email protected] did a bunch of stuff with typedefs that might cause problems and I haven't tested it yet. Anyone with time and Windows is welcome to contribute here! (@No9?)

[rvagg]

aaaaand, I've put [email protected] as a target for this too. It's not released yet but it's going to be necessary to get us back in the 0.11 game. 0.11.11 and 0.11.12 are broken for all native addons, 0.11.13 is going to have some huge changes in V8 and NAN has been catching up.

This is the important part: [email protected] is going to give us 0.11.13 support and most likely make us ready for 0.12 which is just around the corner (v8 will likely be held stable from the next 0.11 release until 0.12). But, it'll break compatibility with the previous 0.11 releases that we've supported. I'm not concerned about this because it's called "unstable" for a reason, but expect bug reports from people trying to make it work.

[rvagg]

Upgraded to [email protected] (git dep for now) and it works on Node master and 0.10.26!

.. except for one test failure. put-get-del-test.js in abstract-leveldown gets passed a Buffer to use in a test case, when trying to use this Buffer as a value (seems to be fine as a key), I get a nasty crash and I can't work out what the problem is.

Failing test is here if anyone wants a challenge. Works fine with 0.10.26, fails on Node master, possibly to do with Buffer changes in core but more likely something to do with LevelDOWN internals and the way its interacting with objects via the new NAN interface.

[kkoopa]

All tests passed with nodejs/node-v0.x-archive@b84ebfe from some time in March. Master fails, so it does seem like a change to something buffer-related must have happened between these two points. These two are buffer-related and fall within that time frame: nodejs/node-v0.x-archive@4c36f3e nodejs/node-v0.x-archive@8e823bc

*** glibc detected *** node: double free or corruption (!prev): 0x000000000124db90 *** ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x7eb96)[0x7f0aa1949b96] /home/kkoopa/dev/node-leveldown/build/Release/leveldown.node(_ZN9leveldown11WriteWorker12WorkCompleteEv+0x9f)[0x7f0aa2be8e8f] /home/kkoopa/dev/node-leveldown/build/Release/leveldown.node(_Z23NanAsyncExecuteCompleteP9uv_work_s+0x15)[0x7f0aa2be1135]

WriteWorker::WorkComplete seems to be the culprit, causing a double free.

[kkoopa]

Removing https://github.com/rvagg/node-leveldown/blob/master/src/leveldown.h#L36 makes the test pass.

[rvagg]

@kkoopa I'm narrowing this down a bit, it's releated to Buffer::HasInstance() not detecting Buffer instances properly. The line you pointed to in leveldown.h shouldn't be reached in this case because it's a buffer it's not deletable but it's detecting it as a buffer. I'm pretty sure I've found at least one bug in Node core, perhaps two (one of them might be V8 related though). Trying to narrow down a simple test case.

[kkoopa]

I suspected that it was related to Buffer::HasInstance(), but node_buffer.cc did not seem like it had been changed between those two commits.

[kkoopa]

This might be because NAN does not set the new isolate argument for buffers, using the old deprecated constructor. I'll fix that.

[kkoopa]

Nope, seems it did not help.

[rvagg]

Good news, I've tracked down the bug to something that's fixable in NAN (nodejs/nan#90), although I'm not sure if it's a NAN problem of a V8 problem or ... but it's fixable in NAN in any case so all good on the Node 0.11.13 front as soon as the next NAN is out!

[rvagg]

updated for [email protected] (just released) so we have Node 0.11.13 support with this branch

[duralog]

I had a look at this for reference to some things for another module I want to convert (nodegit - which is a HUGE/substantial amount of work) and looking over this code dude, there is so much shit that's like way over my head... very impressive work :) wow. btw, this branch compiling fine on mavericks and should be merged. (or at least reference this branch from the nan homepage, instead of the main one which uses 0.6.0... lol)

[juliangruber]

is the solution of #82 required for 0.11?

[kesla]

@rvagg could you perhaps take a look at #101 and #103? With those merged I think that we have a really solid leveldown 0.11

[rvagg]

I'm still hesitating because of #85, it's going to be a major change that would be good to get in a major version bump and I'd rather not have a 2.0.0 follow quickly after a 1.0.0. I'd really like for that to be cleaned up and merged before we release.

I'm also fine with #101 and #103, would like to hear from anyone with the time to code review pre-merge.

[duralog]

the current branch works perfectly for me. very nice.

also, there are a bunch of tars in the deps folder.
I noticed that it was updated to 0.17.0 without the tar.gz being added there.

[kesla]

@duralog AFAIK code.google.com don't offer tar.gz-releases anymore, so there is no official .tar.gz-file to put in that folder. @rvagg would it be ok for you if I removed the old files also or is there some specific reason for them to being in there?

[rvagg]

@kesla yes, their usefulness has expired, clean out all the things!

[rvagg]

I don't have time to look in to this now but there seems to be a missing NanScope() in NextWorker::HandleOKCallback()

[kesla]

@rvagg @kkoopa if I've understood the nan-code correctly, there's no need for a NanScope() in the HandleOkCallback see https://github.com/rvagg/nan/blob/master/nan.h#L1811-L1818

[rvagg]

done a bunch of merging and upgraded to [email protected], I wanted to try and make some real movement towards a release on this but I'm now stuck on a broken build and am going to have to defer to @kesla to see if he can fix it, sorry!

[kesla]

I'm on it!

[kesla]

I've fixed some of the failing tests, but right now the test/ introduced in #110 and then the snapshot-fix in #117 doesn't play so nice with each other (commenting out the snapshot-fix will make the test pass).

@abliss @rvagg anyone of you interested at taking a look at this?

[abliss]

@kesla I can take a look today, but as a possible quick fix you can try increasing the stack-size=128 in iterator-recursion-test.js line 35.

[kesla]

@abliss I've got it, there's a failure in stack-blower.js We're creating the iterators without opening the db first, and now that we create a snapshot when creating the iterator the database must be opened first. I'll push a fix shortly

[kesla]

Sigh... I added it so that we run node 0.11 on travis - and there we have a failing tests. But I can't reproduce it running node 0.11.13 locally (on Mac OS X).

[rvagg]

I can't reproduce the Travis problem either but I'd love to know what the deal is. https://travis-ci.org/rvagg/node-leveldown/jobs/31542070

I've tried running a fresh Ubuntu 12.04 Docker container, installed 0.11.13 and run the tests, so it's just like the Travis environment, to no avail. I'm suspecting it might be to do with the speed of the machine it's running on, perhaps.

[duralog]

long shot: perhaps it's using some other version of tap that times out weird... so, maybe try changing the command instead to ./node_modules/.bin/tap test/*-test.js --stderr will work?

the other thing I guess I'd try is making the setTimeout in the test longer - if it is the machine's speed, as you suggest

I'm really grasping at straws here... it does not reproduce for me on osx/ubuntu either

[rvagg]

Soooo .. 1.0.0 was just published. Thanks for all the good work everyone, particularly @kesla who has the biggest footprint in this release. We need to get a matching [email protected] out but the fstream stuff is blocking it so we can remove WriteStream but retain the functional tests.

Major themes in this release are:

• Really fast iterator thanks for buffering/batching
• Writing of empty values
• Wrapping in abstract-leveldown so we're pushing some of the type-checking out into JS (there may be more work to do here to lessen the C++ load)
• Node 0.11.13+ support

I'll try and find time to write this up, perhaps after a LevelUP release. We also have the forks to do and the level-* wrapper packages.

[dominictarr]

what was the breaking change?

[rvagg]

writing empty values, plus we just needed to get past the 1.0.0 barrier (regularly doing this with all my libs when I change them these days, screw 0.x.y)