fix: leveldb iterator memory leak

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 } })
       })
     }),

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()

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()
+    })
+  })
 })