diff --git a/binding.cc b/binding.cc index 3dbc1bc..b299571 100644 --- a/binding.cc +++ b/binding.cc @@ -18,7 +18,9 @@ * Forward declarations. */ struct Database; +struct Resource; struct Iterator; +struct ExplicitSnapshot; static leveldb::Status threadsafe_open(const leveldb::Options &options, bool multithreading, Database &db_instance); @@ -53,6 +55,10 @@ static std::map db_handles; Batch* batch = NULL; \ NAPI_STATUS_THROWS(napi_get_value_external(env, argv[0], (void**)&batch)); +#define NAPI_SNAPSHOT_CONTEXT() \ + ExplicitSnapshot* snapshot = NULL; \ + NAPI_STATUS_THROWS(napi_get_value_external(env, argv[0], (void**)&snapshot)); + #define NAPI_RETURN_UNDEFINED() \ return 0; @@ -527,14 +533,14 @@ struct BaseWorker { }; /** - * Owns the LevelDB storage, cache, filter policy and iterators. + * Owns the LevelDB storage, cache, filter policy and resources. */ struct Database { Database () : db_(NULL), blockCache_(NULL), filterPolicy_(leveldb::NewBloomFilterPolicy(10)), - currentIteratorId_(0), + resourceSequence_(0), pendingCloseWorker_(NULL), ref_(NULL), priorityWork_(0) {} @@ -611,24 +617,15 @@ struct Database { return db_->ReleaseSnapshot(snapshot); } - void AttachIterator (napi_env env, uint32_t id, Iterator* iterator) { - iterators_[id] = iterator; - IncrementPriorityWork(env); - } - - void DetachIterator (napi_env env, uint32_t id) { - iterators_.erase(id); - DecrementPriorityWork(env); - } - + // The env argument is unused but reminds us to increment priorityWork_ + // only in the JavaScript main thread to avoid needing a lock around + // that and pendingCloseWorker_. void IncrementPriorityWork (napi_env env) { - napi_reference_ref(env, ref_, &priorityWork_); + priorityWork_++; } void DecrementPriorityWork (napi_env env) { - napi_reference_unref(env, ref_, &priorityWork_); - - if (priorityWork_ == 0 && pendingCloseWorker_ != NULL) { + if (--priorityWork_ == 0 && pendingCloseWorker_ != NULL) { pendingCloseWorker_->Queue(env); pendingCloseWorker_ = NULL; } @@ -641,13 +638,13 @@ struct Database { leveldb::DB* db_; leveldb::Cache* blockCache_; const leveldb::FilterPolicy* filterPolicy_; - uint32_t currentIteratorId_; + uint32_t resourceSequence_; BaseWorker *pendingCloseWorker_; - std::map< uint32_t, Iterator * > iterators_; + std::map resources_; napi_ref ref_; private: - uint32_t priorityWork_; + std::atomic priorityWork_; std::string location_; // for separation of concerns the threadsafe functionality was kept at the global @@ -708,6 +705,60 @@ leveldb::Status threadsafe_close(Database &db_instance) { return leveldb::Status::OK(); } +/** + * Represents an object that has a strong reference until explicitly closed. In + * addition, resources are tracked in database->resources in order to close + * them when the Node.js environment is tore down. + */ +struct Resource { + Resource (Database* database) + : database(database), + id_(++database->resourceSequence_), + ref_(NULL) { + } + + virtual ~Resource () { } + virtual void CloseResource () = 0; + + void Attach (napi_env env, napi_value context) { + napi_create_reference(env, context, 1, &ref_); + database->resources_[id_] = this; + } + + void Detach (napi_env env) { + database->resources_.erase(id_); + if (ref_ != NULL) napi_delete_reference(env, ref_); + } + + static void CollectGarbage (napi_env env, void* data, void* hint) { + if (data) { + delete (Resource*)data; + } + } + + Database* database; + +private: + const uint32_t id_; + napi_ref ref_; +}; + +/** + * Explicit snapshot of database. + */ +struct ExplicitSnapshot final : public Resource { + ExplicitSnapshot (Database* database) + : Resource(database), + nut(database->NewSnapshot()) { + } + + void Resource::CloseResource () override { + database->ReleaseSnapshot(nut); + } + + const leveldb::Snapshot* nut; +}; + /** * Base worker class for doing async work that defers closing the database. */ @@ -736,7 +787,8 @@ struct BaseIterator { std::string* gt, std::string* gte, const int limit, - const bool fillCache) + const bool fillCache, + ExplicitSnapshot* snapshot) : database_(database), hasClosed_(false), didSeek_(false), @@ -749,7 +801,15 @@ struct BaseIterator { count_(0) { options_ = new leveldb::ReadOptions(); options_->fill_cache = fillCache; - options_->snapshot = database->NewSnapshot(); + + if (snapshot == NULL) { + implicitSnapshot_ = database_->NewSnapshot(); + options_->snapshot = implicitSnapshot_; + } else { + implicitSnapshot_ = NULL; + options_->snapshot = snapshot->nut; + } + dbIterator_ = database_->NewIterator(options_); } @@ -833,12 +893,16 @@ struct BaseIterator { } } - void Close () { + void CloseIterator () { if (!hasClosed_) { hasClosed_ = true; + delete dbIterator_; dbIterator_ = NULL; - database_->ReleaseSnapshot(options_->snapshot); + + if (implicitSnapshot_) { + database_->ReleaseSnapshot(implicitSnapshot_); + } } } @@ -913,14 +977,14 @@ struct BaseIterator { const int limit_; int count_; leveldb::ReadOptions* options_; + const leveldb::Snapshot* implicitSnapshot_; }; /** * Extends BaseIterator for reading it from JS land. */ -struct Iterator final : public BaseIterator { +struct Iterator final : public BaseIterator, public Resource { Iterator (Database* database, - const uint32_t id, const bool reverse, const bool keys, const bool values, @@ -933,9 +997,10 @@ struct Iterator final : public BaseIterator { const Encoding keyEncoding, const Encoding valueEncoding, const uint32_t highWaterMarkBytes, - unsigned char* state) - : BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache), - id_(id), + unsigned char* state, + ExplicitSnapshot* snapshot) + : BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache, snapshot), + Resource(database), keys_(keys), values_(values), keyEncoding_(keyEncoding), @@ -943,23 +1008,15 @@ struct Iterator final : public BaseIterator { highWaterMarkBytes_(highWaterMarkBytes), first_(true), nexting_(false), - isClosing_(false), aborted_(false), ended_(false), - state_(state), - ref_(NULL) { + state_(state) { } ~Iterator () {} - void Attach (napi_env env, napi_value context) { - napi_create_reference(env, context, 1, &ref_); - database_->AttachIterator(env, id_, this); - } - - void Detach (napi_env env) { - database_->DetachIterator(env, id_); - if (ref_ != NULL) napi_delete_reference(env, ref_); + void Resource::CloseResource () override { + BaseIterator::CloseIterator(); } bool ReadMany (uint32_t size) { @@ -998,7 +1055,6 @@ struct Iterator final : public BaseIterator { return false; } - const uint32_t id_; const bool keys_; const bool values_; const Encoding keyEncoding_; @@ -1006,14 +1062,10 @@ struct Iterator final : public BaseIterator { const uint32_t highWaterMarkBytes_; bool first_; bool nexting_; - bool isClosing_; std::atomic aborted_; bool ended_; unsigned char* state_; std::vector cache_; - -private: - napi_ref ref_; }; /** @@ -1031,15 +1083,10 @@ static void env_cleanup_hook (void* arg) { // where it's our responsibility to clean up. Note also, the following code must // be a safe noop if called before db_open() or after db_close(). if (database && database->db_ != NULL) { - std::map iterators = database->iterators_; - std::map::iterator it; - - // TODO: does not do `napi_delete_reference(env, iterator->ref_)`. Problem? - for (it = iterators.begin(); it != iterators.end(); ++it) { - it->second->Close(); + for (const auto& kv : database->resources_) { + kv.second->CloseResource(); } - // Having closed the iterators (and released snapshots) we can safely close. database->CloseDatabase(); } } @@ -1068,8 +1115,8 @@ NAPI_METHOD(db_init) { FinalizeDatabase, NULL, &result)); - // Reference counter to prevent GC of database while priority workers are active - NAPI_STATUS_THROWS(napi_create_reference(env, result, 0, &database->ref_)); + // Prevent GC of database before close() + NAPI_STATUS_THROWS(napi_create_reference(env, result, 1, &database->ref_)); return result; } @@ -1162,6 +1209,9 @@ NAPI_METHOD(db_open) { /** * Worker class for closing a database + * + * TODO: once we've moved the PriorityWork logic to AbstractLevel, check if + * CloseDatabase is fast enough to be done synchronously. */ struct CloseWorker final : public BaseWorker { CloseWorker (napi_env env, Database* database, napi_deferred deferred) @@ -1172,6 +1222,11 @@ struct CloseWorker final : public BaseWorker { void DoExecute () override { database_->CloseDatabase(); } + + void DoFinally (napi_env env) override { + napi_reference_unref(env, database_->ref_, NULL); + BaseWorker::DoFinally(env); + } }; napi_value noop_callback (napi_env env, napi_callback_info info) { @@ -1186,8 +1241,8 @@ NAPI_METHOD(db_close) { NAPI_DB_CONTEXT(); NAPI_PROMISE(); - // AbstractLevel should not call _close() before iterators are closed - assert(database->iterators_.size() == 0); + // AbstractLevel should not call _close() before resources are closed + assert(database->resources_.size() == 0); CloseWorker* worker = new CloseWorker(env, database, deferred); @@ -1256,12 +1311,20 @@ struct GetWorker final : public PriorityWorker { napi_deferred deferred, leveldb::Slice key, const Encoding encoding, - const bool fillCache) + const bool fillCache, + ExplicitSnapshot* snapshot) : PriorityWorker(env, database, deferred, "classic_level.db.get"), key_(key), encoding_(encoding) { options_.fill_cache = fillCache; - options_.snapshot = database->NewSnapshot(); + + if (snapshot == NULL) { + implicitSnapshot_ = database->NewSnapshot(); + options_.snapshot = implicitSnapshot_; + } else { + implicitSnapshot_ = NULL; + options_.snapshot = snapshot->nut; + } } ~GetWorker () { @@ -1270,7 +1333,10 @@ struct GetWorker final : public PriorityWorker { void DoExecute () override { SetStatus(database_->Get(options_, key_, value_)); - database_->ReleaseSnapshot(options_.snapshot); + + if (implicitSnapshot_) { + database_->ReleaseSnapshot(implicitSnapshot_); + } } void HandleOKCallback (napi_env env, napi_deferred deferred) override { @@ -1284,13 +1350,14 @@ struct GetWorker final : public PriorityWorker { leveldb::Slice key_; std::string value_; const Encoding encoding_; + const leveldb::Snapshot* implicitSnapshot_; }; /** * Gets a value from a database. */ NAPI_METHOD(db_get) { - NAPI_ARGV(4); + NAPI_ARGV(5); NAPI_DB_CONTEXT(); NAPI_PROMISE(); @@ -1298,8 +1365,11 @@ NAPI_METHOD(db_get) { const Encoding encoding = GetEncoding(env, argv[2]); const bool fillCache = BooleanValue(env, argv[3], true); + ExplicitSnapshot* snapshot = NULL; + napi_get_value_external(env, argv[4], (void**)&snapshot); + GetWorker* worker = new GetWorker( - env, database, deferred, key, encoding, fillCache + env, database, deferred, key, encoding, fillCache, snapshot ); worker->Queue(env); @@ -1315,11 +1385,19 @@ struct GetManyWorker final : public PriorityWorker { std::vector keys, napi_deferred deferred, const Encoding valueEncoding, - const bool fillCache) + const bool fillCache, + ExplicitSnapshot* snapshot) : PriorityWorker(env, database, deferred, "classic_level.get.many"), keys_(std::move(keys)), valueEncoding_(valueEncoding) { options_.fill_cache = fillCache; - options_.snapshot = database->NewSnapshot(); + + if (snapshot == NULL) { + implicitSnapshot_ = database->NewSnapshot(); + options_.snapshot = implicitSnapshot_; + } else { + implicitSnapshot_ = NULL; + options_.snapshot = snapshot->nut; + } } void DoExecute () override { @@ -1344,7 +1422,9 @@ struct GetManyWorker final : public PriorityWorker { } } - database_->ReleaseSnapshot(options_.snapshot); + if (implicitSnapshot_) { + database_->ReleaseSnapshot(implicitSnapshot_); + } } void HandleOKCallback (napi_env env, napi_deferred deferred) override { @@ -1368,13 +1448,14 @@ struct GetManyWorker final : public PriorityWorker { const std::vector keys_; const Encoding valueEncoding_; std::vector cache_; + const leveldb::Snapshot* implicitSnapshot_; }; /** * Gets many values from a database. */ NAPI_METHOD(db_get_many) { - NAPI_ARGV(3); + NAPI_ARGV(4); NAPI_DB_CONTEXT(); NAPI_PROMISE(); @@ -1383,8 +1464,17 @@ NAPI_METHOD(db_get_many) { const Encoding valueEncoding = GetEncoding(env, options, "valueEncoding"); const bool fillCache = BooleanProperty(env, options, "fillCache", true); + ExplicitSnapshot* snapshot = NULL; + napi_get_value_external(env, argv[3], (void**)&snapshot); + GetManyWorker* worker = new GetManyWorker( - env, database, keys, deferred, valueEncoding, fillCache + env, + database, + keys, + deferred, + valueEncoding, + fillCache, + snapshot ); worker->Queue(env); @@ -1446,9 +1536,10 @@ struct ClearWorker final : public PriorityWorker { std::string* lt, std::string* lte, std::string* gt, - std::string* gte) + std::string* gte, + ExplicitSnapshot* snapshot) : PriorityWorker(env, database, deferred, "classic_level.db.clear") { - iterator_ = new BaseIterator(database, reverse, lt, lte, gt, gte, limit, false); + iterator_ = new BaseIterator(database, reverse, lt, lte, gt, gte, limit, false, snapshot); writeOptions_ = new leveldb::WriteOptions(); writeOptions_->sync = false; } @@ -1486,7 +1577,7 @@ struct ClearWorker final : public PriorityWorker { batch.Clear(); } - iterator_->Close(); + iterator_->CloseIterator(); } private: @@ -1498,7 +1589,7 @@ struct ClearWorker final : public PriorityWorker { * Delete a range from a database. */ NAPI_METHOD(db_clear) { - NAPI_ARGV(2); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); NAPI_PROMISE(); @@ -1512,8 +1603,11 @@ NAPI_METHOD(db_clear) { std::string* gt = RangeOption(env, options, "gt"); std::string* gte = RangeOption(env, options, "gte"); + ExplicitSnapshot* snapshot = NULL; + napi_get_value_external(env, argv[2], (void**)&snapshot); + ClearWorker* worker = new ClearWorker( - env, database, deferred, reverse, limit, lt, lte, gt, gte + env, database, deferred, reverse, limit, lt, lte, gt, gte, snapshot ); worker->Queue(env); @@ -1703,20 +1797,11 @@ NAPI_METHOD(repair_db) { return promise; } -/** - * Runs when an Iterator is garbage collected. - */ -static void FinalizeIterator (napi_env env, void* data, void* hint) { - if (data) { - delete (Iterator*)data; - } -} - /** * Create an iterator. */ NAPI_METHOD(iterator_init) { - NAPI_ARGV(3); + NAPI_ARGV(4); NAPI_DB_CONTEXT(); unsigned char* state = 0; @@ -1739,19 +1824,27 @@ NAPI_METHOD(iterator_init) { std::string* gt = RangeOption(env, options, "gt"); std::string* gte = RangeOption(env, options, "gte"); - const uint32_t id = database->currentIteratorId_++; - Iterator* iterator = new Iterator(database, id, reverse, keys, - values, limit, lt, lte, gt, gte, fillCache, - keyEncoding, valueEncoding, highWaterMarkBytes, - state); - napi_value result; + ExplicitSnapshot* snapshot = NULL; + napi_get_value_external(env, argv[3], (void**)&snapshot); + + Iterator* iterator = new Iterator( + database, + reverse, + keys, values, + limit, + lt, lte, gt, gte, + fillCache, + keyEncoding, valueEncoding, + highWaterMarkBytes, + state, + snapshot + ); + napi_value result; NAPI_STATUS_THROWS(napi_create_external(env, iterator, - FinalizeIterator, + Resource::CollectGarbage, NULL, &result)); - // Prevent GC of JS object before the iterator is closed (explicitly or on - // db close) and keep track of non-closed iterators to close them on db close. iterator->Attach(env, result); return result; @@ -1765,7 +1858,6 @@ NAPI_METHOD(iterator_seek) { NAPI_ITERATOR_CONTEXT(); // AbstractIterator should not call _seek() after _close() - assert(!iterator->isClosing_); assert(!iterator->hasClosed_); leveldb::Slice target = ToSlice(env, argv[1]); @@ -1777,49 +1869,21 @@ NAPI_METHOD(iterator_seek) { NAPI_RETURN_UNDEFINED(); } -/** - * Worker class for closing an iterator - */ -struct CloseIteratorWorker final : public BaseWorker { - CloseIteratorWorker (napi_env env, Iterator* iterator, napi_deferred deferred) - : BaseWorker(env, iterator->database_, deferred, "classic_level.iterator.close"), - iterator_(iterator) {} - - ~CloseIteratorWorker () {} - - void DoExecute () override { - iterator_->Close(); - } - - void DoFinally (napi_env env) override { - iterator_->Detach(env); - BaseWorker::DoFinally(env); - } - -private: - Iterator* iterator_; -}; - /** * Closes an iterator. */ NAPI_METHOD(iterator_close) { NAPI_ARGV(1); NAPI_ITERATOR_CONTEXT(); - NAPI_PROMISE(); - // AbstractIterator should not call _close() more than once - assert(!iterator->isClosing_); + // AbstractIterator should not call _close() more than once or while nexting assert(!iterator->hasClosed_); - - // AbstractIterator should not call _close() while next() is in-progress assert(!iterator->nexting_); - CloseIteratorWorker* worker = new CloseIteratorWorker(env, iterator, deferred); - iterator->isClosing_ = true; - worker->Queue(env); + iterator->CloseResource(); + iterator->Detach(env); - return promise; + NAPI_RETURN_UNDEFINED(); } /** @@ -1837,7 +1901,7 @@ NAPI_METHOD(iterator_abort) { */ struct NextWorker final : public BaseWorker { NextWorker (napi_env env, Iterator* iterator, uint32_t size, napi_deferred deferred) - : BaseWorker(env, iterator->database_, deferred, "classic_level.iterator.next"), + : BaseWorker(env, iterator->database, deferred, "classic_level.iterator.next"), iterator_(iterator), size_(size), ok_() {} ~NextWorker () {} @@ -1909,7 +1973,6 @@ NAPI_METHOD(iterator_nextv) { if (size == 0) size = 1; // AbstractIterator should not call _next() or _nextv() after _close() - assert(!iterator->isClosing_); assert(!iterator->hasClosed_); if (iterator->ended_) { @@ -2172,6 +2235,41 @@ NAPI_METHOD(batch_write) { return promise; } +/** + * Create a snapshot context. + */ +NAPI_METHOD(snapshot_init) { + NAPI_ARGV(1); + NAPI_DB_CONTEXT(); + + ExplicitSnapshot* snapshot = new ExplicitSnapshot(database); + + napi_value context; + NAPI_STATUS_THROWS(napi_create_external( + env, + snapshot, + Resource::CollectGarbage, + NULL, + &context + )); + + snapshot->Attach(env, context); + return context; +} + +/** + * Closes the snapshot. + */ +NAPI_METHOD(snapshot_close) { + NAPI_ARGV(1); + NAPI_SNAPSHOT_CONTEXT(); + + snapshot->CloseResource(); + snapshot->Detach(env); + + NAPI_RETURN_UNDEFINED(); +} + /** * All exported functions. */ @@ -2203,4 +2301,7 @@ NAPI_INIT() { NAPI_EXPORT_FUNCTION(batch_del); NAPI_EXPORT_FUNCTION(batch_clear); NAPI_EXPORT_FUNCTION(batch_write); + + NAPI_EXPORT_FUNCTION(snapshot_init); + NAPI_EXPORT_FUNCTION(snapshot_close); } diff --git a/index.js b/index.js index fdb3332..af74d61 100644 --- a/index.js +++ b/index.js @@ -1,6 +1,6 @@ 'use strict' -const { AbstractLevel } = require('abstract-level') +const { AbstractLevel, AbstractSnapshot } = require('abstract-level') const ModuleError = require('module-error') const fsp = require('fs/promises') const binding = require('./binding') @@ -25,6 +25,7 @@ class ClassicLevel extends AbstractLevel { seek: true, createIfMissing: true, errorIfExists: true, + explicitSnapshots: true, additionalMethods: { approximateSize: true, compactRange: true @@ -63,12 +64,18 @@ class ClassicLevel extends AbstractLevel { this[kContext], key, encodingEnum(options.valueEncoding), - options.fillCache + options.fillCache, + options.snapshot?.[kContext] ) } async _getMany (keys, options) { - return binding.db_get_many(this[kContext], keys, options) + return binding.db_get_many( + this[kContext], + keys, + options, + options.snapshot?.[kContext] + ) } async _del (key, options) { @@ -76,7 +83,11 @@ class ClassicLevel extends AbstractLevel { } async _clear (options) { - return binding.db_clear(this[kContext], options) + return binding.db_clear( + this[kContext], + options, + options.snapshot?.[kContext] + ) } _chainedBatch () { @@ -145,7 +156,16 @@ class ClassicLevel extends AbstractLevel { } _iterator (options) { - return new Iterator(this, this[kContext], options) + return new Iterator( + this, + this[kContext], + options, + options.snapshot?.[kContext] + ) + } + + _snapshot (options) { + return new Snapshot(this[kContext], options) } static async destroy (location) { @@ -165,6 +185,19 @@ class ClassicLevel extends AbstractLevel { } } +// Defined here so that both ClassicLevel and Snapshot can access kContext +class Snapshot extends AbstractSnapshot { + constructor (context, options) { + super(options) + this[kContext] = binding.snapshot_init(context) + } + + async _close () { + // This is synchronous because that's faster than creating async work + binding.snapshot_close(this[kContext]) + } +} + exports.ClassicLevel = ClassicLevel // It's faster to read options in JS than to pass options objects to C++. diff --git a/iterator.js b/iterator.js index cc8d29d..44c196b 100644 --- a/iterator.js +++ b/iterator.js @@ -21,11 +21,11 @@ const STATE_ENDED = 1 // so it'll typically just make one nextv(1000) call and there's // no performance gain in overriding _all(). class Iterator extends AbstractIterator { - constructor (db, context, options) { + constructor (db, context, options, snapshotCtx) { super(db, options) this[kState] = new Uint8Array(1) - this[kContext] = binding.iterator_init(context, this[kState], options) + this[kContext] = binding.iterator_init(context, this[kState], options, snapshotCtx) this[kFirst] = true this[kCache] = empty this[kPosition] = 0 @@ -104,7 +104,8 @@ class Iterator extends AbstractIterator { this[kSignal] = null } - return binding.iterator_close(this[kContext]) + // This is synchronous because that's faster than creating async work + binding.iterator_close(this[kContext]) } [kAbort] () { diff --git a/package.json b/package.json index 02584bc..1df02e0 100644 --- a/package.json +++ b/package.json @@ -27,7 +27,7 @@ "prebuild-win32-x64": "prebuildify -t 18.20.4 --napi --strip" }, "dependencies": { - "abstract-level": "^2.0.0", + "abstract-level": "github:Level/abstract-level#explicit-snapshots", "module-error": "^1.0.1", "napi-macros": "^2.2.2", "node-gyp-build": "^4.3.0" diff --git a/test/iterator-gc-test.js b/test/iterator-gc-test.js index 0f3bb60..5bfbedc 100644 --- a/test/iterator-gc-test.js +++ b/test/iterator-gc-test.js @@ -1,50 +1,74 @@ -'use strict' - -const test = require('tape') -const testCommon = require('./common') -const sourceData = [] - -for (let i = 0; i < 1e3; i++) { - sourceData.push({ - type: 'put', - key: i.toString(), - value: Math.random().toString() - }) -} - -// When you have a database open with an active iterator, but no references to -// the db, V8 will GC the database and you'll get an failed assert from LevelDB. -test('db without ref does not get GCed while iterating', async function (t) { - let db = testCommon.factory() - - await db.open() - - // Insert test data - await db.batch(sourceData.slice()) - - // Set highWaterMarkBytes to 0 so that we don't preemptively fetch. - const it = db.iterator({ highWaterMarkBytes: 0 }) - - // Remove reference - db = null - - if (global.gc) { - // This is the reliable way to trigger GC (and the bug if it exists). - // Useful for manual testing with "node --expose-gc". - global.gc() - } else { - // But a timeout usually also allows GC to kick in. If not, the time - // between iterator ticks might. That's when "highWaterMarkBytes: 0" helps. - await new Promise(resolve => setTimeout(resolve, 1e3)) - } - - // No reference to db here, could be GCed. It shouldn't.. - const entries = await it.all() - t.is(entries.length, sourceData.length, 'got data') - - // Because we also have a reference on the iterator. That's the fix. - t.ok(it.db, 'abstract iterator has reference to db') - - // Which as luck would have it, also allows us to properly end this test. - return it.db.close() -}) +'use strict' + +const test = require('tape') +const testCommon = require('./common') +const sourceData = [] + +for (let i = 0; i < 1e3; i++) { + sourceData.push({ + type: 'put', + key: i.toString(), + value: Math.random().toString() + }) +} + +// When you have a database open with an active iterator, but no references to +// the db, V8 will GC the database and you'll get an failed assert from LevelDB. +test('db without ref does not get GCed while iterating', async function (t) { + let db = testCommon.factory() + + await db.open() + + // Insert test data + await db.batch(sourceData.slice()) + + // Set highWaterMarkBytes to 0 so that we don't preemptively fetch. + const it = db.iterator({ highWaterMarkBytes: 0 }) + + // Remove reference + db = null + + if (global.gc) { + // This is the reliable way to trigger GC (and the bug if it exists). + // Useful for manual testing with "node --expose-gc". + global.gc() + } else { + // But a timeout usually also allows GC to kick in. If not, the time + // between iterator ticks might. That's when "highWaterMarkBytes: 0" helps. + await new Promise(resolve => setTimeout(resolve, 1e3)) + } + + // No reference to db here, could be GCed. It shouldn't.. + const entries = await it.all() + t.is(entries.length, sourceData.length, 'got data') + + // Because we also have a reference on the iterator. That's the fix. + t.ok(it.db, 'abstract iterator has reference to db') + + // Which as luck would have it, also allows us to properly end this test. + return it.db.close() +}) + +// Same as above but also nullifying the iterator +test('db and iterator without ref does not get GCed while iterating', async function (t) { + let db = testCommon.factory() + + await db.open() + await db.batch(sourceData.slice()) + + let it = db.iterator({ + highWaterMarkBytes: sourceData.length * 32 + }) + + t.is((await it.nextv(1000)).length, sourceData.length, 'got data') + + // Remove references + it = null + db = null + + if (global.gc) { + global.gc() + } else { + await new Promise(resolve => setTimeout(resolve, 1e3)) + } +})