Nullptr dereference in test/iterator-recursion-test.js

[ianwjhalliday]

There is a crash when running test/iterator-recursion-test.js due to trying to open the db in the child process (stack-blower.js) while the db is already open in the test process.

leveldown::Database::OpenDatabase does not handle error conditions from leveldb::DB::Open. In this case Open fails due to the LOCK file already being held by the parent process. As a result, the out parameter is set to (or left at) nullptr. Later on the call to iterator() from the test script causes an access on the Database::db field and crashes.

I can't see how this test would have worked ever. However, I am pretty sure this used to work fine months ago.

Here is a simple repro I have verified on Windows 10 Anniversary Update, Node.js v6.3.1, npm 3.10.3, and leveldown 1.4.6:

const leveldown = require('leveldown')
    , cp = require('child_process');

let db = leveldown('foodb');
db.open(function () {
    if (process.argv[2] != '2') {
        let child = cp.fork(__filename, [ '2' ]);
        child.on('exit', function (code) {
            console.log(code.toString(16));
        });
    } else {
        db.iterator({ start: '0' });
    }
    db.close(function () { });
});

[rvagg]

Yeah, I discovered this too. I guess the try/catch is actually catching something else. I already fixed it in #302 which has been merged in to #299 and will be in the next update which could be any day now.

[rvagg]

Also, fwiw I found it while migrating the NewInstance stuff to use MaybeLocals and I'm guessing you found this via a similar route when testing napi stuff.