diff --git a/src/index.js b/src/index.js index a3760f7..7ac8bab 100644 --- a/src/index.js +++ b/src/index.js @@ -158,7 +158,12 @@ function levelIteratorToIterator (li) { next: () => new Promise((resolve, reject) => { li.next((err, key, value) => { if (err) return reject(err) - if (key == null) return resolve({ done: true }) + if (key == null) { + return li.end(err => { + if (err) return reject(err) + resolve({ done: true }) + }) + } resolve({ done: false, value: { key, value } }) }) }), diff --git a/test/fixtures/test-level-iterator-destroy.js b/test/fixtures/test-level-iterator-destroy.js new file mode 100644 index 0000000..a71a3d5 --- /dev/null +++ b/test/fixtures/test-level-iterator-destroy.js @@ -0,0 +1,18 @@ +'use strict' + +const { utils } = require('interface-datastore') +const LevelStore = require('../../src') + +async function testLevelIteratorDestroy () { + const store = new LevelStore(utils.tmpdir(), { db: require('level') }) + await store.open() + await store.put(`/test/key${Date.now()}`, Buffer.from(`TESTDATA${Date.now()}`)) + for await (const d of store.query({})) { + console.log(d) // eslint-disable-line no-console + } +} + +// Will exit with: +// > Assertion failed: (ended_), function ~Iterator, file ../binding.cc, line 546. +// If iterator gets destroyed (in c++ land) and .end() was not called on it. +testLevelIteratorDestroy() diff --git a/test/node.js b/test/node.js index 9c11ad0..2cf0115 100644 --- a/test/node.js +++ b/test/node.js @@ -10,6 +10,7 @@ const rimraf = require('rimraf') const { MountDatastore } = require('datastore-core') const CID = require('cids') const { promisify } = require('util') +const childProcess = require('child_process') const LevelStore = require('../src') @@ -68,4 +69,33 @@ describe('LevelDatastore', () => { expect(cids[0].version).to.be.eql(0) expect(cids).to.have.length(4) }) + + // The `.end()` method MUST be called on LevelDB iterators or they remain open, + // leaking memory. + // + // This test exposes this problem by causing an error to be thrown on process + // exit when an iterator is open AND leveldb is not closed. + // + // Normally when leveldb is closed it'll automatically clean up open iterators + // but if you don't close the store this error will occur: + // + // > Assertion failed: (ended_), function ~Iterator, file ../binding.cc, line 546. + // + // This is thrown by a destructor function for iterator objects that asserts + // the iterator has ended before cleanup. + // + // https://github.com/Level/leveldown/blob/d3453fbde4d2a8aa04d9091101c25c999649069b/binding.cc#L545 + it('should not leave iterators open and leak memory', (done) => { + const cp = childProcess.fork(`${__dirname}/fixtures/test-level-iterator-destroy`, { stdio: 'pipe' }) + + let out = '' + cp.stdout.on('data', d => { out += d }) + cp.stderr.on('data', d => { out += d }) + + cp.on('exit', code => { + expect(code).to.equal(0) + expect(out).to.not.include('Assertion failed: (ended_)') + done() + }) + }) })