From d5bad805ce19a9d8aac7bb573b286bf75f3ee091 Mon Sep 17 00:00:00 2001 From: Vincent Weevers Date: Fri, 18 Nov 2022 13:54:41 +0100 Subject: [PATCH] Breaking: remove callbacks and `LEVEL_NOT_FOUND` Category: change --- binding.cc | 425 +++++++++++++------------ chained-batch.js | 7 +- index.d.ts | 23 +- index.js | 98 +++--- iterator.js | 91 +++--- package.json | 2 - test/approximate-size-test.js | 70 ++-- test/chained-batch-gc-test.js | 39 +-- test/cleanup-hanging-iterators-test.js | 146 +++++---- test/clear-gc-test.js | 38 +-- test/compact-range-test.js | 76 ++--- test/compression-test.js | 109 ++++--- test/destroy-test.js | 77 ++--- test/env-cleanup-hook-test.js | 4 +- test/env-cleanup-hook.js | 48 ++- test/getproperty-test.js | 20 +- test/iterator-gc-test.js | 67 ++-- test/iterator-hwm-test.js | 1 + test/iterator-recursion-test.js | 80 ++--- test/iterator-starvation-test.js | 162 +++++----- test/iterator-test.js | 74 +++-- test/leak-tester-batch.js | 39 +-- test/leak-tester-iterator.js | 51 ++- test/leak-tester.js | 60 ++-- test/lock-test.js | 30 +- test/lock.js | 4 +- test/make.js | 31 +- test/repair-test.js | 64 ++-- test/segfault-test.js | 78 ++--- test/stack-blower.js | 27 +- 30 files changed, 949 insertions(+), 1092 deletions(-) diff --git a/binding.cc b/binding.cc index 015b283f..27fb7cd5 100644 --- a/binding.cc +++ b/binding.cc @@ -18,7 +18,6 @@ */ struct Database; struct Iterator; -static void iterator_close_do (napi_env env, Iterator* iterator, napi_value cb); static leveldb::Status threadsafe_open(const leveldb::Options &options, bool multithreading, Database &db_instance); @@ -56,6 +55,11 @@ static std::map db_handles; #define NAPI_RETURN_UNDEFINED() \ return 0; +#define NAPI_PROMISE() \ + napi_deferred deferred; \ + napi_value promise; \ + NAPI_STATUS_THROWS(napi_create_promise(env, &deferred, &promise)); + #define NAPI_UTF8_NEW(name, val) \ size_t name##_size = 0; \ NAPI_STATUS_THROWS(napi_get_value_string_utf8(env, val, NULL, 0, &name##_size)) \ @@ -94,6 +98,11 @@ static std::map db_handles; } \ } +/** + * Bit fields. + */ +#define STATE_ENDED 1 + /********************************************************************* * Helpers. ********************************************************************/ @@ -311,18 +320,6 @@ static std::vector KeyArray (napi_env env, napi_value arr) { return result; } -/** - * Calls a function. - */ -static napi_status CallFunction (napi_env env, - napi_value callback, - const int argc, - napi_value* argv) { - napi_value global; - napi_get_global(env, &global); - return napi_call_function(env, global, callback, argc, argv, NULL); -} - /** * Whether to yield entries, keys or values. */ @@ -379,23 +376,26 @@ struct Entry { * following virtual methods (listed in the order in which they're called): * * - DoExecute (abstract, worker pool thread): main work - * - HandleOKCallback (main thread): call JS callback on success - * - HandleErrorCallback (main thread): call JS callback on error + * - HandleOKCallback (main thread): resolve JS promise on success + * - HandleErrorCallback (main thread): reject JS promise on error * - DoFinally (main thread): do cleanup regardless of success */ struct BaseWorker { // Note: storing env is discouraged as we'd end up using it in unsafe places. BaseWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, const char* resourceName) - : database_(database), errMsg_(NULL) { - NAPI_STATUS_THROWS_VOID(napi_create_reference(env, callback, 1, &callbackRef_)); + : database_(database), errMsg_(NULL), deferred_(deferred) { + // Note: napi_deferred is a strong reference to the JS promise, so there's no need to + // create a reference ourselves. See `v8_deferred = new v8::Persistent()` in: + // https://github.com/nodejs/node/commit/7efb8f7619100973877c660d0ee527ea3d92de8d + napi_value asyncResourceName; NAPI_STATUS_THROWS_VOID(napi_create_string_utf8(env, resourceName, NAPI_AUTO_LENGTH, &asyncResourceName)); - NAPI_STATUS_THROWS_VOID(napi_create_async_work(env, callback, + NAPI_STATUS_THROWS_VOID(napi_create_async_work(env, NULL, asyncResourceName, BaseWorker::Execute, BaseWorker::Complete, @@ -440,28 +440,29 @@ struct BaseWorker { } void DoComplete (napi_env env) { - napi_value callback; - napi_get_reference_value(env, callbackRef_, &callback); - if (status_.ok()) { - HandleOKCallback(env, callback); + HandleOKCallback(env, deferred_); } else { - HandleErrorCallback(env, callback); + HandleErrorCallback(env, deferred_); } } - virtual void HandleOKCallback (napi_env env, napi_value callback) { + virtual void HandleOKCallback (napi_env env, napi_deferred deferred) { napi_value argv; - napi_get_null(env, &argv); - CallFunction(env, callback, 1, &argv); + napi_get_undefined(env, &argv); + napi_resolve_deferred(env, deferred, argv); } - virtual void HandleErrorCallback (napi_env env, napi_value callback) { + virtual void HandleErrorCallback (napi_env env, napi_deferred deferred) { napi_value argv; if (status_.IsNotFound()) { - argv = CreateCodeError(env, "LEVEL_NOT_FOUND", errMsg_); - } else if (status_.IsCorruption()) { + napi_get_undefined(env, &argv); + napi_resolve_deferred(env, deferred, argv); + return; + } + + if (status_.IsCorruption()) { argv = CreateCodeError(env, "LEVEL_CORRUPTION", errMsg_); } else if (status_.IsIOError()) { if (strlen(errMsg_) > 15 && strncmp("IO error: lock ", errMsg_, 15) == 0) { // env_posix.cc @@ -475,13 +476,12 @@ struct BaseWorker { argv = CreateError(env, errMsg_); } - CallFunction(env, callback, 1, &argv); + napi_reject_deferred(env, deferred, argv); } virtual void DoFinally (napi_env env) { - napi_delete_reference(env, callbackRef_); napi_delete_async_work(env, asyncWork_); - + deferred_ = NULL; delete this; } @@ -492,7 +492,7 @@ struct BaseWorker { Database* database_; private: - napi_ref callbackRef_; + napi_deferred deferred_; napi_async_work asyncWork_; leveldb::Status status_; char *errMsg_; @@ -653,7 +653,7 @@ leveldb::Status threadsafe_open(const leveldb::Options &options, db_instance.db_ = handle.db; db_handles[db_instance.location_] = handle; } - + return status; } @@ -684,8 +684,8 @@ leveldb::Status threadsafe_close(Database &db_instance) { * Base worker class for doing async work that defers closing the database. */ struct PriorityWorker : public BaseWorker { - PriorityWorker (napi_env env, Database* database, napi_value callback, const char* resourceName) - : BaseWorker(env, database, callback, resourceName) { + PriorityWorker (napi_env env, Database* database, napi_deferred deferred, const char* resourceName) + : BaseWorker(env, database, deferred, resourceName) { database_->IncrementPriorityWork(env); } @@ -904,7 +904,8 @@ struct Iterator final : public BaseIterator { const bool fillCache, const Encoding keyEncoding, const Encoding valueEncoding, - const uint32_t highWaterMarkBytes) + const uint32_t highWaterMarkBytes, + unsigned char* state) : BaseIterator(database, reverse, lt, lte, gt, gte, limit, fillCache), id_(id), keys_(keys), @@ -915,7 +916,9 @@ struct Iterator final : public BaseIterator { first_(true), nexting_(false), isClosing_(false), + ended_(false), closeWorker_(NULL), + state_(state), ref_(NULL) { } @@ -963,6 +966,7 @@ struct Iterator final : public BaseIterator { } } + ended_ = true; return false; } @@ -975,7 +979,9 @@ struct Iterator final : public BaseIterator { bool first_; bool nexting_; bool isClosing_; + bool ended_; BaseWorker* closeWorker_; + unsigned char* state_; std::vector cache_; private: @@ -1047,7 +1053,7 @@ NAPI_METHOD(db_init) { struct OpenWorker final : public BaseWorker { OpenWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, const std::string& location, const bool createIfMissing, const bool errorIfExists, @@ -1058,7 +1064,7 @@ struct OpenWorker final : public BaseWorker { const uint32_t maxOpenFiles, const uint32_t blockRestartInterval, const uint32_t maxFileSize) - : BaseWorker(env, database, callback, "classic_level.db.open"), + : BaseWorker(env, database, deferred, "classic_level.db.open"), location_(location), multithreading_(multithreading) { options_.block_cache = database->blockCache_; @@ -1090,7 +1096,7 @@ struct OpenWorker final : public BaseWorker { * Open a database. */ NAPI_METHOD(db_open) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); NAPI_ARGV_UTF8_NEW(location, 1); @@ -1110,27 +1116,29 @@ NAPI_METHOD(db_open) { database->blockCache_ = leveldb::NewLRUCache(cacheSize); - napi_value callback = argv[3]; - OpenWorker* worker = new OpenWorker(env, database, callback, location, - createIfMissing, errorIfExists, - compression, multithreading, - writeBufferSize, blockSize, - maxOpenFiles, blockRestartInterval, - maxFileSize); + NAPI_PROMISE(); + + OpenWorker* worker = new OpenWorker( + env, database, deferred, location, + createIfMissing, errorIfExists, + compression, multithreading, + writeBufferSize, blockSize, + maxOpenFiles, blockRestartInterval, + maxFileSize + ); + worker->Queue(env); delete [] location; - NAPI_RETURN_UNDEFINED(); + return promise; } /** * Worker class for closing a database */ struct CloseWorker final : public BaseWorker { - CloseWorker (napi_env env, - Database* database, - napi_value callback) - : BaseWorker(env, database, callback, "classic_level.db.close") {} + CloseWorker (napi_env env, Database* database, napi_deferred deferred) + : BaseWorker(env, database, deferred, "classic_level.db.close") {} ~CloseWorker () {} @@ -1147,30 +1155,43 @@ napi_value noop_callback (napi_env env, napi_callback_info info) { * Close a database. */ NAPI_METHOD(db_close) { - NAPI_ARGV(2); + NAPI_ARGV(1); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); - napi_value callback = argv[1]; - CloseWorker* worker = new CloseWorker(env, database, callback); + CloseWorker* worker = new CloseWorker(env, database, deferred); if (!database->HasPriorityWork()) { worker->Queue(env); - NAPI_RETURN_UNDEFINED(); + return promise; } database->pendingCloseWorker_ = worker; - napi_value noop; - napi_create_function(env, NULL, 0, noop_callback, NULL, &noop); - std::map iterators = database->iterators_; std::map::iterator it; for (it = iterators.begin(); it != iterators.end(); ++it) { - iterator_close_do(env, it->second, noop); + Iterator* iterator = it->second; + + // TODO (v2): should no longer be necessary? Also not in abstract-level v1. But + // if it is, then find a cleaner solution than creating a whole bunch of throwaway promises. + if (!iterator->isClosing_ && !iterator->hasClosed_) { + // napi_value iteratorPromise; + // napi_deferred iteratorDeferred; + // NAPI_STATUS_THROWS(napi_create_promise(env, &iteratorDeferred, &iteratorPromise)); + // CloseIteratorWorker* worker = new CloseIteratorWorker(env, iterator, iteratorPromise, iteratorDeferred); + // iterator->isClosing_ = true; + // + // if (iterator->nexting_) { + // iterator->closeWorker_ = worker; + // } else { + // worker->Queue(env); + // } + } } - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1179,11 +1200,11 @@ NAPI_METHOD(db_close) { struct PutWorker final : public PriorityWorker { PutWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::Slice key, leveldb::Slice value, bool sync) - : PriorityWorker(env, database, callback, "classic_level.db.put"), + : PriorityWorker(env, database, deferred, "classic_level.db.put"), key_(key), value_(value) { options_.sync = sync; } @@ -1206,18 +1227,18 @@ struct PutWorker final : public PriorityWorker { * Puts a key and a value to a database. */ NAPI_METHOD(db_put) { - NAPI_ARGV(5); + NAPI_ARGV(4); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); leveldb::Slice key = ToSlice(env, argv[1]); leveldb::Slice value = ToSlice(env, argv[2]); bool sync = BooleanProperty(env, argv[3], "sync", false); - napi_value callback = argv[4]; - PutWorker* worker = new PutWorker(env, database, callback, key, value, sync); + PutWorker* worker = new PutWorker(env, database, deferred, key, value, sync); worker->Queue(env); - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1226,11 +1247,11 @@ NAPI_METHOD(db_put) { struct GetWorker final : public PriorityWorker { GetWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::Slice key, const Encoding encoding, const bool fillCache) - : PriorityWorker(env, database, callback, "classic_level.db.get"), + : PriorityWorker(env, database, deferred, "classic_level.db.get"), key_(key), encoding_(encoding) { options_.fill_cache = fillCache; @@ -1244,11 +1265,10 @@ struct GetWorker final : public PriorityWorker { SetStatus(database_->Get(options_, key_, value_)); } - void HandleOKCallback (napi_env env, napi_value callback) override { - napi_value argv[2]; - napi_get_null(env, &argv[0]); - Entry::Convert(env, &value_, encoding_, argv[1]); - CallFunction(env, callback, 2, argv); + void HandleOKCallback (napi_env env, napi_deferred deferred) override { + napi_value argv; + Entry::Convert(env, &value_, encoding_, argv); + napi_resolve_deferred(env, deferred, argv); } private: @@ -1262,20 +1282,21 @@ struct GetWorker final : public PriorityWorker { * Gets a value from a database. */ NAPI_METHOD(db_get) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); leveldb::Slice key = ToSlice(env, argv[1]); napi_value options = argv[2]; const Encoding encoding = GetEncoding(env, options, "valueEncoding"); const bool fillCache = BooleanProperty(env, options, "fillCache", true); - napi_value callback = argv[3]; - GetWorker* worker = new GetWorker(env, database, callback, key, encoding, - fillCache); - worker->Queue(env); + GetWorker* worker = new GetWorker( + env, database, deferred, key, encoding, fillCache + ); - NAPI_RETURN_UNDEFINED(); + worker->Queue(env); + return promise; } /** @@ -1285,10 +1306,10 @@ struct GetManyWorker final : public PriorityWorker { GetManyWorker (napi_env env, Database* database, std::vector keys, - napi_value callback, + napi_deferred deferred, const Encoding valueEncoding, const bool fillCache) - : PriorityWorker(env, database, callback, "classic_level.get.many"), + : PriorityWorker(env, database, deferred, "classic_level.get.many"), keys_(std::move(keys)), valueEncoding_(valueEncoding) { options_.fill_cache = fillCache; options_.snapshot = database->NewSnapshot(); @@ -1319,7 +1340,7 @@ struct GetManyWorker final : public PriorityWorker { database_->ReleaseSnapshot(options_.snapshot); } - void HandleOKCallback (napi_env env, napi_value callback) override { + void HandleOKCallback (napi_env env, napi_deferred deferred) override { size_t size = cache_.size(); napi_value array; napi_create_array_with_length(env, size, &array); @@ -1332,10 +1353,7 @@ struct GetManyWorker final : public PriorityWorker { if (value != NULL) delete value; } - napi_value argv[2]; - napi_get_null(env, &argv[0]); - argv[1] = array; - CallFunction(env, callback, 2, argv); + napi_resolve_deferred(env, deferred, array); } private: @@ -1349,21 +1367,21 @@ struct GetManyWorker final : public PriorityWorker { * Gets many values from a database. */ NAPI_METHOD(db_get_many) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); const auto keys = KeyArray(env, argv[1]); napi_value options = argv[2]; const Encoding valueEncoding = GetEncoding(env, options, "valueEncoding"); const bool fillCache = BooleanProperty(env, options, "fillCache", true); - napi_value callback = argv[3]; GetManyWorker* worker = new GetManyWorker( - env, database, keys, callback, valueEncoding, fillCache + env, database, keys, deferred, valueEncoding, fillCache ); worker->Queue(env); - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1372,10 +1390,10 @@ NAPI_METHOD(db_get_many) { struct DelWorker final : public PriorityWorker { DelWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::Slice key, bool sync) - : PriorityWorker(env, database, callback, "classic_level.db.del"), + : PriorityWorker(env, database, deferred, "classic_level.db.del"), key_(key) { options_.sync = sync; } @@ -1396,17 +1414,17 @@ struct DelWorker final : public PriorityWorker { * Delete a value from a database. */ NAPI_METHOD(db_del) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); leveldb::Slice key = ToSlice(env, argv[1]); bool sync = BooleanProperty(env, argv[2], "sync", false); - napi_value callback = argv[3]; - DelWorker* worker = new DelWorker(env, database, callback, key, sync); + DelWorker* worker = new DelWorker(env, database, deferred, key, sync); worker->Queue(env); - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1415,14 +1433,14 @@ NAPI_METHOD(db_del) { struct ClearWorker final : public PriorityWorker { ClearWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, const bool reverse, const int limit, std::string* lt, std::string* lte, std::string* gt, std::string* gte) - : PriorityWorker(env, database, callback, "classic_level.db.clear") { + : PriorityWorker(env, database, deferred, "classic_level.db.clear") { iterator_ = new BaseIterator(database, reverse, lt, lte, gt, gte, limit, false); writeOptions_ = new leveldb::WriteOptions(); writeOptions_->sync = false; @@ -1473,11 +1491,11 @@ struct ClearWorker final : public PriorityWorker { * Delete a range from a database. */ NAPI_METHOD(db_clear) { - NAPI_ARGV(3); + NAPI_ARGV(2); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); napi_value options = argv[1]; - napi_value callback = argv[2]; const bool reverse = BooleanProperty(env, options, "reverse", false); const int limit = Int32Property(env, options, "limit", -1); @@ -1487,10 +1505,12 @@ NAPI_METHOD(db_clear) { std::string* gt = RangeOption(env, options, "gt"); std::string* gte = RangeOption(env, options, "gte"); - ClearWorker* worker = new ClearWorker(env, database, callback, reverse, limit, lt, lte, gt, gte); - worker->Queue(env); + ClearWorker* worker = new ClearWorker( + env, database, deferred, reverse, limit, lt, lte, gt, gte + ); - NAPI_RETURN_UNDEFINED(); + worker->Queue(env); + return promise; } /** @@ -1499,10 +1519,10 @@ NAPI_METHOD(db_clear) { struct ApproximateSizeWorker final : public PriorityWorker { ApproximateSizeWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::Slice start, leveldb::Slice end) - : PriorityWorker(env, database, callback, "classic_level.db.approximate_size"), + : PriorityWorker(env, database, deferred, "classic_level.db.approximate_size"), start_(start), end_(end) {} ~ApproximateSizeWorker () { @@ -1515,11 +1535,10 @@ struct ApproximateSizeWorker final : public PriorityWorker { size_ = database_->ApproximateSize(&range); } - void HandleOKCallback (napi_env env, napi_value callback) override { - napi_value argv[2]; - napi_get_null(env, &argv[0]); - napi_create_int64(env, (uint64_t)size_, &argv[1]); - CallFunction(env, callback, 2, argv); + void HandleOKCallback (napi_env env, napi_deferred deferred) override { + napi_value argv; + napi_create_int64(env, (uint64_t)size_, &argv); + napi_resolve_deferred(env, deferred, argv); } leveldb::Slice start_; @@ -1531,20 +1550,19 @@ struct ApproximateSizeWorker final : public PriorityWorker { * Calculates the approximate size of a range in a database. */ NAPI_METHOD(db_approximate_size) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); leveldb::Slice start = ToSlice(env, argv[1]); leveldb::Slice end = ToSlice(env, argv[2]); - napi_value callback = argv[3]; + ApproximateSizeWorker* worker = new ApproximateSizeWorker( + env, database, deferred, start, end + ); - ApproximateSizeWorker* worker = new ApproximateSizeWorker(env, database, - callback, start, - end); worker->Queue(env); - - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1553,10 +1571,10 @@ NAPI_METHOD(db_approximate_size) { struct CompactRangeWorker final : public PriorityWorker { CompactRangeWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::Slice start, leveldb::Slice end) - : PriorityWorker(env, database, callback, "classic_level.db.compact_range"), + : PriorityWorker(env, database, deferred, "classic_level.db.compact_range"), start_(start), end_(end) {} ~CompactRangeWorker () { @@ -1576,18 +1594,19 @@ struct CompactRangeWorker final : public PriorityWorker { * Compacts a range in a database. */ NAPI_METHOD(db_compact_range) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); leveldb::Slice start = ToSlice(env, argv[1]); leveldb::Slice end = ToSlice(env, argv[2]); - napi_value callback = argv[3]; - CompactRangeWorker* worker = new CompactRangeWorker(env, database, callback, - start, end); - worker->Queue(env); + CompactRangeWorker* worker = new CompactRangeWorker( + env, database, deferred, start, end + ); - NAPI_RETURN_UNDEFINED(); + worker->Queue(env); + return promise; } /** @@ -1614,10 +1633,8 @@ NAPI_METHOD(db_get_property) { * Worker class for destroying a database. */ struct DestroyWorker final : public BaseWorker { - DestroyWorker (napi_env env, - const std::string& location, - napi_value callback) - : BaseWorker(env, NULL, callback, "classic_level.destroy_db"), + DestroyWorker (napi_env env, const std::string& location, napi_deferred deferred) + : BaseWorker(env, NULL, deferred, "classic_level.destroy_db"), location_(location) {} ~DestroyWorker () {} @@ -1634,26 +1651,23 @@ struct DestroyWorker final : public BaseWorker { * Destroys a database. */ NAPI_METHOD(destroy_db) { - NAPI_ARGV(2); + NAPI_ARGV(1); NAPI_ARGV_UTF8_NEW(location, 0); - napi_value callback = argv[1]; + NAPI_PROMISE(); - DestroyWorker* worker = new DestroyWorker(env, location, callback); + DestroyWorker* worker = new DestroyWorker(env, location, deferred); worker->Queue(env); delete [] location; - - NAPI_RETURN_UNDEFINED(); + return promise; } /** * Worker class for repairing a database. */ struct RepairWorker final : public BaseWorker { - RepairWorker (napi_env env, - const std::string& location, - napi_value callback) - : BaseWorker(env, NULL, callback, "classic_level.repair_db"), + RepairWorker (napi_env env, const std::string& location, napi_deferred deferred) + : BaseWorker(env, NULL, deferred, "classic_level.repair_db"), location_(location) {} ~RepairWorker () {} @@ -1670,16 +1684,16 @@ struct RepairWorker final : public BaseWorker { * Repairs a database. */ NAPI_METHOD(repair_db) { - NAPI_ARGV(2); + NAPI_ARGV(1); NAPI_ARGV_UTF8_NEW(location, 0); - napi_value callback = argv[1]; + NAPI_PROMISE(); - RepairWorker* worker = new RepairWorker(env, location, callback); + RepairWorker* worker = new RepairWorker(env, location, deferred); worker->Queue(env); delete [] location; - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1695,10 +1709,15 @@ static void FinalizeIterator (napi_env env, void* data, void* hint) { * Create an iterator. */ NAPI_METHOD(iterator_init) { - NAPI_ARGV(2); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); - napi_value options = argv[1]; + unsigned char* state = 0; + size_t stateLength; + NAPI_STATUS_THROWS(napi_get_typedarray_info(env, argv[1], NULL, &stateLength, (void**)&state, NULL, NULL)); + assert(stateLength == 1); + + napi_value options = argv[2]; const bool reverse = BooleanProperty(env, options, "reverse", false); const bool keys = BooleanProperty(env, options, "keys", true); const bool values = BooleanProperty(env, options, "values", true); @@ -1716,7 +1735,8 @@ NAPI_METHOD(iterator_init) { const uint32_t id = database->currentIteratorId_++; Iterator* iterator = new Iterator(database, id, reverse, keys, values, limit, lt, lte, gt, gte, fillCache, - keyEncoding, valueEncoding, highWaterMarkBytes); + keyEncoding, valueEncoding, highWaterMarkBytes, + state); napi_value result; NAPI_STATUS_THROWS(napi_create_external(env, iterator, @@ -1743,6 +1763,7 @@ NAPI_METHOD(iterator_seek) { leveldb::Slice target = ToSlice(env, argv[1]); iterator->first_ = true; + iterator->ended_ = false; iterator->Seek(target); DisposeSliceBuffer(target); @@ -1753,10 +1774,8 @@ NAPI_METHOD(iterator_seek) { * Worker class for closing an iterator */ struct CloseIteratorWorker final : public BaseWorker { - CloseIteratorWorker (napi_env env, - Iterator* iterator, - napi_value callback) - : BaseWorker(env, iterator->database_, callback, "classic_level.iterator.close"), + CloseIteratorWorker (napi_env env, Iterator* iterator, napi_deferred deferred) + : BaseWorker(env, iterator->database_, deferred, "classic_level.iterator.close"), iterator_(iterator) {} ~CloseIteratorWorker () {} @@ -1775,12 +1794,15 @@ struct CloseIteratorWorker final : public BaseWorker { }; /** - * Called by NAPI_METHOD(iterator_close) and also when closing - * open iterators during NAPI_METHOD(db_close). + * Closes an iterator. */ -static void iterator_close_do (napi_env env, Iterator* iterator, napi_value cb) { +NAPI_METHOD(iterator_close) { + NAPI_ARGV(1); + NAPI_ITERATOR_CONTEXT(); + NAPI_PROMISE(); + if (!iterator->isClosing_ && !iterator->hasClosed_) { - CloseIteratorWorker* worker = new CloseIteratorWorker(env, iterator, cb); + CloseIteratorWorker* worker = new CloseIteratorWorker(env, iterator, deferred); iterator->isClosing_ = true; if (iterator->nexting_) { @@ -1788,31 +1810,21 @@ static void iterator_close_do (napi_env env, Iterator* iterator, napi_value cb) } else { worker->Queue(env); } + } else { + // TODO (v2): I think we can remove the isClosing_ checks + napi_value argv = CreateCodeError(env, "LEVEL_XYZ", "Should not happen?"); + NAPI_STATUS_THROWS(napi_reject_deferred(env, deferred, argv)); } -} - -/** - * Closes an iterator. - */ -NAPI_METHOD(iterator_close) { - NAPI_ARGV(2); - NAPI_ITERATOR_CONTEXT(); - - iterator_close_do(env, iterator, argv[1]); - NAPI_RETURN_UNDEFINED(); + return promise; } /** * Worker class for nexting an iterator. */ struct NextWorker final : public BaseWorker { - NextWorker (napi_env env, - Iterator* iterator, - uint32_t size, - napi_value callback) - : BaseWorker(env, iterator->database_, callback, - "classic_level.iterator.next"), + NextWorker (napi_env env, Iterator* iterator, uint32_t size, napi_deferred deferred) + : BaseWorker(env, iterator->database_, deferred, "classic_level.iterator.next"), iterator_(iterator), size_(size), ok_() {} ~NextWorker () {} @@ -1829,7 +1841,7 @@ struct NextWorker final : public BaseWorker { } } - void HandleOKCallback (napi_env env, napi_value callback) override { + void HandleOKCallback (napi_env env, napi_deferred deferred) override { size_t size = iterator_->cache_.size(); napi_value jsArray; napi_create_array_with_length(env, size, &jsArray); @@ -1843,17 +1855,19 @@ struct NextWorker final : public BaseWorker { napi_set_element(env, jsArray, idx, element); } - napi_value argv[3]; - napi_get_null(env, &argv[0]); - argv[1] = jsArray; - napi_get_boolean(env, !ok_, &argv[2]); - CallFunction(env, callback, 3, argv); + // TODO: use state_ internally too, replacing ended_? + if (iterator_->ended_) { + *iterator_->state_ |= STATE_ENDED; + } + + napi_resolve_deferred(env, deferred, jsArray); } void DoFinally (napi_env env) override { // clean up & handle the next/close state iterator_->nexting_ = false; + // TODO (v2): check if still needed if (iterator_->closeWorker_ != NULL) { iterator_->closeWorker_->Queue(env); iterator_->closeWorker_ = NULL; @@ -1872,26 +1886,29 @@ struct NextWorker final : public BaseWorker { * Advance repeatedly and get multiple entries at once. */ NAPI_METHOD(iterator_nextv) { - NAPI_ARGV(3); + NAPI_ARGV(2); NAPI_ITERATOR_CONTEXT(); + NAPI_PROMISE(); uint32_t size; NAPI_STATUS_THROWS(napi_get_value_uint32(env, argv[1], &size)); if (size == 0) size = 1; - napi_value callback = argv[2]; - if (iterator->isClosing_ || iterator->hasClosed_) { + // TODO (v2): test, or remove napi_value argv = CreateCodeError(env, "LEVEL_ITERATOR_NOT_OPEN", "Iterator is not open"); - NAPI_STATUS_THROWS(CallFunction(env, callback, 1, &argv)); - NAPI_RETURN_UNDEFINED(); + NAPI_STATUS_THROWS(napi_reject_deferred(env, deferred, argv)); + } else if (iterator->ended_) { + napi_value empty; + napi_create_array_with_length(env, 0, &empty); + napi_resolve_deferred(env, deferred, empty); + } else { + NextWorker* worker = new NextWorker(env, iterator, size, deferred); + iterator->nexting_ = true; + worker->Queue(env); } - NextWorker* worker = new NextWorker(env, iterator, size, callback); - iterator->nexting_ = true; - worker->Queue(env); - - NAPI_RETURN_UNDEFINED(); + return promise; } /** @@ -1900,11 +1917,11 @@ NAPI_METHOD(iterator_nextv) { struct BatchWorker final : public PriorityWorker { BatchWorker (napi_env env, Database* database, - napi_value callback, + napi_deferred deferred, leveldb::WriteBatch* batch, const bool sync, const bool hasData) - : PriorityWorker(env, database, callback, "classic_level.batch.do"), + : PriorityWorker(env, database, deferred, "classic_level.batch.do"), batch_(batch), hasData_(hasData) { options_.sync = sync; } @@ -1929,12 +1946,12 @@ struct BatchWorker final : public PriorityWorker { * Does a batch write operation on a database. */ NAPI_METHOD(batch_do) { - NAPI_ARGV(4); + NAPI_ARGV(3); NAPI_DB_CONTEXT(); + NAPI_PROMISE(); napi_value array = argv[1]; const bool sync = BooleanProperty(env, argv[2], "sync", false); - napi_value callback = argv[3]; uint32_t length; napi_get_array_length(env, array, &length); @@ -1973,10 +1990,12 @@ NAPI_METHOD(batch_do) { } } - BatchWorker* worker = new BatchWorker(env, database, callback, batch, sync, hasData); - worker->Queue(env); + BatchWorker* worker = new BatchWorker( + env, database, deferred, batch, sync, hasData + ); - NAPI_RETURN_UNDEFINED(); + worker->Queue(env); + return promise; } /** @@ -2092,9 +2111,9 @@ struct BatchWriteWorker final : public PriorityWorker { BatchWriteWorker (napi_env env, napi_value context, Batch* batch, - napi_value callback, + napi_deferred deferred, const bool sync) - : PriorityWorker(env, batch->database_, callback, "classic_level.batch.write"), + : PriorityWorker(env, batch->database_, deferred, "classic_level.batch.write"), batch_(batch), sync_(sync) { // Prevent GC of batch object before we execute @@ -2124,17 +2143,19 @@ struct BatchWriteWorker final : public PriorityWorker { * Writes a batch object. */ NAPI_METHOD(batch_write) { - NAPI_ARGV(3); + NAPI_ARGV(2); NAPI_BATCH_CONTEXT(); + NAPI_PROMISE(); napi_value options = argv[1]; const bool sync = BooleanProperty(env, options, "sync", false); - napi_value callback = argv[2]; - BatchWriteWorker* worker = new BatchWriteWorker(env, argv[0], batch, callback, sync); - worker->Queue(env); + BatchWriteWorker* worker = new BatchWriteWorker( + env, argv[0], batch, deferred, sync + ); - NAPI_RETURN_UNDEFINED(); + worker->Queue(env); + return promise; } /** diff --git a/chained-batch.js b/chained-batch.js index ac343f14..b464dd44 100644 --- a/chained-batch.js +++ b/chained-batch.js @@ -23,13 +23,12 @@ class ChainedBatch extends AbstractChainedBatch { binding.batch_clear(this[kContext]) } - _write (options, callback) { - binding.batch_write(this[kContext], options, callback) + async _write (options) { + return binding.batch_write(this[kContext], options) } - _close (callback) { + async _close () { // TODO: close native batch (currently done on GC) - process.nextTick(callback) } } diff --git a/index.d.ts b/index.d.ts index 4f3cfb01..3d1398b0 100644 --- a/index.d.ts +++ b/index.d.ts @@ -16,8 +16,7 @@ import { AbstractKeyIteratorOptions, AbstractValueIterator, AbstractValueIteratorOptions, - Transcoder, - NodeCallback + Transcoder } from 'abstract-level' /** @@ -48,33 +47,21 @@ declare class ClassicLevel open (): Promise open (options: OpenOptions): Promise - open (callback: NodeCallback): void - open (options: OpenOptions, callback: NodeCallback): void get (key: KDefault): Promise - get (key: KDefault, callback: NodeCallback): void get (key: K, options: GetOptions): Promise - get (key: K, options: GetOptions, callback: NodeCallback): void getMany (keys: KDefault[]): Promise - getMany (keys: KDefault[], callback: NodeCallback): void getMany (keys: K[], options: GetManyOptions): Promise - getMany (keys: K[], options: GetManyOptions, callback: NodeCallback): void put (key: KDefault, value: VDefault): Promise - put (key: KDefault, value: VDefault, callback: NodeCallback): void put (key: K, value: V, options: PutOptions): Promise - put (key: K, value: V, options: PutOptions, callback: NodeCallback): void del (key: KDefault): Promise - del (key: KDefault, callback: NodeCallback): void del (key: K, options: DelOptions): Promise - del (key: K, options: DelOptions, callback: NodeCallback): void batch (operations: Array>): Promise - batch (operations: Array>, callback: NodeCallback): void batch (operations: Array>, options: BatchOptions): Promise - batch (operations: Array>, options: BatchOptions, callback: NodeCallback): void batch (): ChainedBatch iterator (): Iterator @@ -91,17 +78,13 @@ declare class ClassicLevel * `[start..end)`. */ approximateSize (start: KDefault, end: KDefault): Promise - approximateSize (start: KDefault, end: KDefault, callback: NodeCallback): void approximateSize (start: K, end: K, options: StartEndOptions): Promise - approximateSize (start: K, end: K, options: StartEndOptions, callback: NodeCallback): void /** * Manually trigger a database compaction in the range `[start..end)`. */ compactRange (start: KDefault, end: KDefault): Promise - compactRange (start: KDefault, end: KDefault, callback: NodeCallback): void compactRange (start: K, end: K, options: StartEndOptions): Promise - compactRange (start: K, end: K, options: StartEndOptions, callback: NodeCallback): void /** * Get internal details from LevelDB. @@ -113,14 +96,12 @@ declare class ClassicLevel * place of a full directory removal to only remove LevelDB-related files. */ static destroy (location: string): Promise - static destroy (location: string, callback: NodeCallback): void /** * Attempt a restoration of a damaged database. Can also be used to perform * a compaction of the LevelDB log into table files. */ static repair (location: string): Promise - static repair (location: string, callback: NodeCallback): void } /** @@ -311,8 +292,6 @@ export interface ChainedBatchWriteOptions extends AbstractChainedBatchWriteOptio export class ChainedBatch extends AbstractChainedBatch { write (): Promise write (options: ChainedBatchWriteOptions): Promise - write (callback: NodeCallback): void - write (options: ChainedBatchWriteOptions, callback: NodeCallback): void } /** diff --git a/index.js b/index.js index 32a098dc..fc377525 100644 --- a/index.js +++ b/index.js @@ -2,19 +2,18 @@ const { AbstractLevel } = require('abstract-level') const ModuleError = require('module-error') -const { fromCallback } = require('catering') -const fs = require('fs') +const fsp = require('fs/promises') const binding = require('./binding') const { ChainedBatch } = require('./chained-batch') const { Iterator } = require('./iterator') -const kPromise = Symbol('promise') const kContext = Symbol('context') const kLocation = Symbol('location') class ClassicLevel extends AbstractLevel { constructor (location, options, _) { // To help migrating to abstract-level + // TODO (v2): remove if (typeof options === 'function' || typeof _ === 'function') { throw new ModuleError('The levelup-style callback argument has been removed', { code: 'LEVEL_LEGACY' @@ -48,103 +47,88 @@ class ClassicLevel extends AbstractLevel { return this[kLocation] } - _open (options, callback) { + async _open (options) { if (options.createIfMissing) { - fs.mkdir(this[kLocation], { recursive: true }, (err) => { - if (err) return callback(err) - binding.db_open(this[kContext], this[kLocation], options, callback) - }) - } else { - binding.db_open(this[kContext], this[kLocation], options, callback) + await fsp.mkdir(this[kLocation], { recursive: true }) } + + return binding.db_open(this[kContext], this[kLocation], options) } - _close (callback) { - binding.db_close(this[kContext], callback) + async _close () { + return binding.db_close(this[kContext]) } - _put (key, value, options, callback) { - binding.db_put(this[kContext], key, value, options, callback) + async _put (key, value, options) { + return binding.db_put(this[kContext], key, value, options) } - _get (key, options, callback) { - binding.db_get(this[kContext], key, options, callback) + async _get (key, options) { + return binding.db_get(this[kContext], key, options) } - _getMany (keys, options, callback) { - binding.db_get_many(this[kContext], keys, options, callback) + async _getMany (keys, options) { + return binding.db_get_many(this[kContext], keys, options) } - _del (key, options, callback) { - binding.db_del(this[kContext], key, options, callback) + async _del (key, options) { + return binding.db_del(this[kContext], key, options) } - _clear (options, callback) { - binding.db_clear(this[kContext], options, callback) + async _clear (options) { + return binding.db_clear(this[kContext], options) } _chainedBatch () { return new ChainedBatch(this, this[kContext]) } - _batch (operations, options, callback) { - binding.batch_do(this[kContext], operations, options, callback) + async _batch (operations, options) { + return binding.batch_do(this[kContext], operations, options) } - approximateSize (start, end, options, callback) { - if (arguments.length < 2 || typeof start === 'function' || typeof end === 'function') { + // TODO (v2): update docs + async approximateSize (start, end, options) { + if (arguments.length < 2) { throw new TypeError("The arguments 'start' and 'end' are required") - } else if (typeof options === 'function') { - callback = options - options = null } else if (typeof options !== 'object') { options = null } - callback = fromCallback(callback, kPromise) - if (this.status === 'opening') { - this.defer(() => this.approximateSize(start, end, options, callback)) + return this.deferAsync(() => this.approximateSize(start, end, options)) } else if (this.status !== 'open') { - this.nextTick(callback, new ModuleError('Database is not open: cannot call approximateSize()', { + throw new ModuleError('Database is not open: cannot call approximateSize()', { code: 'LEVEL_DATABASE_NOT_OPEN' - })) + }) } else { const keyEncoding = this.keyEncoding(options && options.keyEncoding) start = keyEncoding.encode(start) end = keyEncoding.encode(end) - binding.db_approximate_size(this[kContext], start, end, callback) + return binding.db_approximate_size(this[kContext], start, end) } - - return callback[kPromise] } - compactRange (start, end, options, callback) { - if (arguments.length < 2 || typeof start === 'function' || typeof end === 'function') { + // TODO (v2): update docs + compactRange (start, end, options) { + if (arguments.length < 2) { throw new TypeError("The arguments 'start' and 'end' are required") - } else if (typeof options === 'function') { - callback = options - options = null } else if (typeof options !== 'object') { options = null } - callback = fromCallback(callback, kPromise) - if (this.status === 'opening') { - this.defer(() => this.compactRange(start, end, options, callback)) + return this.deferAsync(() => this.compactRange(start, end, options)) } else if (this.status !== 'open') { - this.nextTick(callback, new ModuleError('Database is not open: cannot call compactRange()', { + throw new ModuleError('Database is not open: cannot call compactRange()', { code: 'LEVEL_DATABASE_NOT_OPEN' - })) + }) } else { const keyEncoding = this.keyEncoding(options && options.keyEncoding) start = keyEncoding.encode(start) end = keyEncoding.encode(end) - binding.db_compact_range(this[kContext], start, end, callback) + return binding.db_compact_range(this[kContext], start, end) } - - return callback[kPromise] } getProperty (property) { @@ -166,24 +150,22 @@ class ClassicLevel extends AbstractLevel { return new Iterator(this, this[kContext], options) } - static destroy (location, callback) { + // TODO (v2): update docs + static async destroy (location) { if (typeof location !== 'string' || location === '') { throw new TypeError("The first argument 'location' must be a non-empty string") } - callback = fromCallback(callback, kPromise) - binding.destroy_db(location, callback) - return callback[kPromise] + return binding.destroy_db(location) } - static repair (location, callback) { + // TODO (v2): update docs + static async repair (location) { if (typeof location !== 'string' || location === '') { throw new TypeError("The first argument 'location' must be a non-empty string") } - callback = fromCallback(callback, kPromise) - binding.repair_db(location, callback) - return callback[kPromise] + return binding.repair_db(location) } } diff --git a/iterator.js b/iterator.js index 26212c7f..f40618c3 100644 --- a/iterator.js +++ b/iterator.js @@ -5,14 +5,16 @@ const binding = require('./binding') const kContext = Symbol('context') const kCache = Symbol('cache') -const kFinished = Symbol('finished') const kFirst = Symbol('first') const kPosition = Symbol('position') -const kHandleNext = Symbol('handleNext') -const kHandleNextv = Symbol('handleNextv') -const kCallback = Symbol('callback') +const kState = Symbol('state') +const kSignal = Symbol('signal') +const kAbort = Symbol('abort') const empty = [] +// Bit fields +const STATE_ENDED = 1 + // Does not implement _all() because the default implementation // of abstract-level falls back to nextv(1000) and using all() // on more entries than that probably isn't a realistic use case, @@ -22,79 +24,64 @@ class Iterator extends AbstractIterator { constructor (db, context, options) { super(db, options) - this[kContext] = binding.iterator_init(context, options) - this[kHandleNext] = this[kHandleNext].bind(this) - this[kHandleNextv] = this[kHandleNextv].bind(this) - this[kCallback] = null + this[kState] = new Uint8Array(1) + this[kContext] = binding.iterator_init(context, this[kState], options) this[kFirst] = true this[kCache] = empty - this[kFinished] = false this[kPosition] = 0 } _seek (target, options) { this[kFirst] = true this[kCache] = empty - this[kFinished] = false + this[kState][0] &= ~STATE_ENDED // Unset this[kPosition] = 0 binding.iterator_seek(this[kContext], target) } - _next (callback) { + async _next () { if (this[kPosition] < this[kCache].length) { - const entry = this[kCache][this[kPosition]++] - process.nextTick(callback, null, entry[0], entry[1]) - } else if (this[kFinished]) { - process.nextTick(callback) - } else { - this[kCallback] = callback - - if (this[kFirst]) { - // It's common to only want one entry initially or after a seek() - this[kFirst] = false - binding.iterator_nextv(this[kContext], 1, this[kHandleNext]) - } else { - // Limit the size of the cache to prevent starving the event loop - // while we're recursively calling process.nextTick(). - binding.iterator_nextv(this[kContext], 1000, this[kHandleNext]) - } + return this[kCache][this[kPosition]++] } - } - [kHandleNext] (err, items, finished) { - const callback = this[kCallback] - if (err) return callback(err) + // Avoid iterator_nextv() call if end was already reached + if ((this[kState][0] & STATE_ENDED) !== 0) { + return undefined + } - this[kCache] = items - this[kFinished] = finished - this[kPosition] = 0 + if (this[kFirst]) { + // It's common to only want one entry initially or after a seek() + this[kFirst] = false + this[kCache] = await binding.iterator_nextv(this[kContext], 1) + this[kPosition] = 0 + } else { + // Limit the size of the cache to prevent starving the event loop + // while we're recursively nexting. + this[kCache] = await binding.iterator_nextv(this[kContext], 1000) + this[kPosition] = 0 + } - this._next(callback) + if (this[kPosition] < this[kCache].length) { + return this[kCache][this[kPosition]++] + } } - _nextv (size, options, callback) { - if (this[kFinished]) { - process.nextTick(callback, null, []) - } else { - this[kCallback] = callback - this[kFirst] = false - binding.iterator_nextv(this[kContext], size, this[kHandleNextv]) + // TODO (v2): read from cache first? + async _nextv (size, options) { + this[kFirst] = false + + // Avoid iterator_nextv() call if end was already reached + if ((this[kState][0] & STATE_ENDED) !== 0) { + return [] } - } - [kHandleNextv] (err, items, finished) { - const callback = this[kCallback] - if (err) return callback(err) - this[kFinished] = finished - callback(null, items) + return binding.iterator_nextv(this[kContext], size) } - _close (callback) { + async _close () { this[kCache] = empty - this[kCallback] = null - - binding.iterator_close(this[kContext], callback) + return binding.iterator_close(this[kContext]) } // Undocumented, exposed for tests only diff --git a/package.json b/package.json index bdb1536b..96766c00 100644 --- a/package.json +++ b/package.json @@ -35,9 +35,7 @@ "devDependencies": { "@types/node": "^18.0.0", "@voxpelli/tsconfig": "^4.0.0", - "async-each": "^1.0.3", "cross-env": "^7.0.3", - "delayed": "^2.0.0", "dependency-check": "^4.1.0", "du": "^1.0.0", "electron": "^21.0.1", diff --git a/test/approximate-size-test.js b/test/approximate-size-test.js index a4e242e3..152e8790 100644 --- a/test/approximate-size-test.js +++ b/test/approximate-size-test.js @@ -2,65 +2,59 @@ const test = require('tape') const testCommon = require('./common') -const noop = () => {} let db -test('setUp db', function (t) { +test('approximateSize() setup', async function (t) { db = testCommon.factory() - db.open(t.end.bind(t)) + return db.open() }) -test('test approximateSize() throws if arguments are missing', function (t) { - for (const args of [[], ['foo'], [noop], ['foo', noop]]) { - t.throws(() => db.approximateSize(...args), { - name: 'TypeError', - message: "The arguments 'start' and 'end' are required" - }) +test('approximateSize() throws if arguments are missing', async function (t) { + t.plan(2 * 2) + + for (const args of [[], ['foo']]) { + try { + await db.approximateSize(...args) + } catch (err) { + t.is(err.name, 'TypeError') + t.is(err.message, "The arguments 'start' and 'end' are required") + } } - t.end() }) -test('test approximateSize()', function (t) { +test('approximateSize()', async function (t) { const data = Array.apply(null, Array(10000)).map(function () { return 'aaaaaaaaaa' }).join('') - db.batch(Array.apply(null, Array(10)).map(function (x, i) { + await db.batch(Array.apply(null, Array(10)).map(function (x, i) { return { type: 'put', key: 'foo' + i, value: data } - }), function (err) { - t.error(err) - - // cycle open/close to ensure a pack to .sst + })) - db.close(function (err) { - t.error(err) - - db.open(function (err) { - t.error(err) + // cycle open/close to ensure a pack to .sst + await db.close() + await db.open() - db.approximateSize('!', '~', function (err, size) { - t.error(err) + const size = await db.approximateSize('!', '~') - t.equal(typeof size, 'number') - // account for snappy compression, original would be ~100000 - t.ok(size > 40000, 'size reports a reasonable amount (' + size + ')') - t.end() - }) - }) - }) - }) + t.equal(typeof size, 'number') + // account for snappy compression, original would be ~100000 + t.ok(size > 40000, 'size reports a reasonable amount (' + size + ')') }) -test('tearDown', function (t) { - db.close(t.end.bind(t)) +test('approximateSize() teardown', async function (t) { + return db.close() }) -test('test approximateSize() yields error if db is closed', function (t) { - db.approximateSize('foo', 'foo', function (err) { - t.is(err && err.code, 'LEVEL_DATABASE_NOT_OPEN') - t.end() - }) +test('approximateSize() yields error if db is closed', async function (t) { + t.plan(1) + + try { + await db.approximateSize('foo', 'foo') + } catch (err) { + t.is(err.code, 'LEVEL_DATABASE_NOT_OPEN') + } }) test('test approximateSize() is deferred', async function (t) { diff --git a/test/chained-batch-gc-test.js b/test/chained-batch-gc-test.js index 1981efcc..4f05de91 100644 --- a/test/chained-batch-gc-test.js +++ b/test/chained-batch-gc-test.js @@ -5,33 +5,28 @@ const testCommon = require('./common') // When we have a chained batch object without a reference, V8 might GC it // before we get a chance to (asynchronously) write the batch. -test('chained batch without ref does not get GCed before write', function (t) { - t.plan(2) - +test('chained batch without ref does not get GCed before write', async function (t) { const db = testCommon.factory() + await db.open() - db.open(function (err) { - t.ifError(err, 'no open error') + let batch = db.batch() - let batch = db.batch() + for (let i = 0; i < 1e3; i++) { + batch.put(String(i), 'value') + } - for (let i = 0; i < 1e3; i++) { - batch.put(String(i), 'value') - } + // The sync option makes the operation slower and thus more likely to + // cause a segfault (if the batch were to be GC-ed before it is written). + const promise = batch.write({ sync: true }) - // The sync option makes the operation slower and thus more likely to - // cause a segfault (if the batch were to be GC-ed before it is written). - batch.write({ sync: true }, function (err) { - t.ifError(err, 'no error from write()') - }) + // Remove reference + batch = null - // Remove reference - batch = 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() + } - 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() - } - }) + return promise }) diff --git a/test/cleanup-hanging-iterators-test.js b/test/cleanup-hanging-iterators-test.js index dc627882..c7d6d182 100644 --- a/test/cleanup-hanging-iterators-test.js +++ b/test/cleanup-hanging-iterators-test.js @@ -3,116 +3,142 @@ const makeTest = require('./make') const repeats = 200 -makeTest('test closed iterator', function (db, t, done) { +makeTest('closed iterator', async function (db, t) { // First test normal and proper usage: calling it.close() before db.close() const it = db.iterator() - - it.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key, 'one', 'correct key') - t.equal(value, '1', 'correct value') - it.close(function (err) { - t.ifError(err, 'no error from close()') - done() - }) - }) + t.same(await it.next(), ['a', '1'], 'correct entry') + await it.close() + return db.close() }) -makeTest('test likely-closed iterator', function (db, t, done) { +makeTest('likely-closed iterator', async function (db, t) { // Test improper usage: not calling it.close() before db.close(). Cleanup of the // database will crash Node if not handled properly. const it = db.iterator() - it.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key, 'one', 'correct key') - t.equal(value, '1', 'correct value') - done() - }) + // Two calls are needed to populate the cache + t.same(await it.next(), ['a', '1'], 'correct entry (1)') + t.same(await it.next(), ['b', '2'], 'correct entry (2)') + + return db.close() }) -makeTest('test non-closed iterator', function (db, t, done) { +makeTest('non-closed iterator', async function (db, t) { // Same as the test above but with a highWaterMarkBytes of 0 so that we don't // preemptively fetch all records, to ensure that the iterator is still // active when we (attempt to) close the database. const it = db.iterator({ highWaterMarkBytes: 0 }) - it.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key, 'one', 'correct key') - t.equal(value, '1', 'correct value') - done() - }) + t.same(await it.next(), ['a', '1'], 'correct entry (1)') + t.same(await it.next(), ['b', '2'], 'correct entry (2)') + + return db.close() }) -makeTest('test multiple likely-closed iterators', function (db, t, done) { +makeTest('non-closed iterator without caching', async function (db, t) { + const it = db.iterator({ highWaterMarkBytes: 0 }) + t.same(await it.next(), ['a', '1'], 'correct entry (1)') + return db.close() +}) + +makeTest('multiple likely-closed iterators', async function (db, t) { // Same as the test above but repeated and with an extra iterator that is not - // nexting, which means its EndWorker will be executed almost immediately. + // nexting, which means its CloseWorker will be executed almost immediately. for (let i = 0; i < repeats; i++) { db.iterator() - db.iterator().next(function () {}) + db.iterator().next() } - setTimeout(done, Math.floor(Math.random() * 50)) + // Avoid async/await to avoid introducing an extra tick + return new Promise((resolve, reject) => { + setTimeout(() => { + db.close().then(resolve, reject) + }, Math.floor(Math.random() * 50)) + }) }) -makeTest('test multiple non-closed iterators', function (db, t, done) { +makeTest('multiple non-closed iterators', async function (db, t) { // Same as the test above but with a highWaterMarkBytes of 0. for (let i = 0; i < repeats; i++) { db.iterator({ highWaterMarkBytes: 0 }) - db.iterator({ highWaterMarkBytes: 0 }).next(function () {}) + db.iterator({ highWaterMarkBytes: 0 }).next() } - setTimeout(done, Math.floor(Math.random() * 50)) + // Avoid async/await to avoid introducing an extra tick + return new Promise((resolve, reject) => { + setTimeout(() => { + db.close().then(resolve, reject) + }, Math.floor(Math.random() * 50)) + }) }) -global.gc && makeTest('test multiple non-closed iterators with forced gc', function (db, t, done) { +global.gc && makeTest('multiple non-closed iterators with forced gc', async function (db, t) { // Same as the test above but with forced GC, to test that the lifespan of an // iterator is tied to *both* its JS object and whether the iterator was closed. for (let i = 0; i < repeats; i++) { db.iterator({ highWaterMarkBytes: 0 }) - db.iterator({ highWaterMarkBytes: 0 }).next(function () {}) + db.iterator({ highWaterMarkBytes: 0 }).next() } - setTimeout(function () { - global.gc() - done() - }, Math.floor(Math.random() * 50)) + // Avoid async/await to avoid introducing an extra tick + return new Promise((resolve, reject) => { + setTimeout(() => { + global.gc() + db.close().then(resolve, reject) + }, Math.floor(Math.random() * 50)) + }) }) -makeTest('test closing iterators', function (db, t, done) { - // At least one end() should be in progress when we try to close the db. - const it1 = db.iterator() - it1.next(function () { - it1.close(function () {}) - }) - const it2 = db.iterator() - it2.next(function () { - it2.close(function () {}) - done() +makeTest('closing iterators', async function (db, t) { + return new Promise((resolve, reject) => { + // At least one close() should be in progress when we try to close the db. + const it1 = db.iterator() + it1.next().then(function () { + it1.close() + }) + + const it2 = db.iterator() + it2.next().then(function () { + it2.close() + db.close().then(resolve, reject) + }) }) }) -makeTest('test recursive next', function (db, t, done) { +makeTest('recursive next', async function (db, t) { // Test that we're able to close when user keeps scheduling work const it = db.iterator({ highWaterMarkBytes: 0 }) - it.next(function loop (err, key) { - if (err && err.code !== 'LEVEL_ITERATOR_NOT_OPEN') throw err - if (key !== undefined) it.next(loop) - }) + function resolve (entry) { + if (entry !== undefined) it.next().then(resolve, reject) + } + + function reject (err) { + if (err.code !== 'LEVEL_ITERATOR_NOT_OPEN') throw err + } - done() + it.next().then(resolve, reject) + return db.close() }) -makeTest('test recursive next (random)', function (db, t, done) { +makeTest('recursive next (random)', async function (db, t) { // Same as the test above but closing at a random time const it = db.iterator({ highWaterMarkBytes: 0 }) - it.next(function loop (err, key) { - if (err && err.code !== 'LEVEL_ITERATOR_NOT_OPEN') throw err - if (key !== undefined) it.next(loop) - }) + function resolve (entry) { + if (entry !== undefined) it.next().then(resolve, reject) + } + + function reject (err) { + if (err.code !== 'LEVEL_ITERATOR_NOT_OPEN') throw err + } + + it.next().then(resolve, reject) - setTimeout(done, Math.floor(Math.random() * 50)) + // Avoid async/await to avoid introducing an extra tick + return new Promise((resolve, reject) => { + setTimeout(() => { + db.close().then(resolve, reject) + }, Math.floor(Math.random() * 50)) + }) }) diff --git a/test/clear-gc-test.js b/test/clear-gc-test.js index 2794878a..abfc9977 100644 --- a/test/clear-gc-test.js +++ b/test/clear-gc-test.js @@ -12,36 +12,22 @@ for (let i = 0; i < 1e3; i++) { }) } -test('db without ref does not get GCed while clear() is in progress', function (t) { - t.plan(4) - +test('db without ref does not get GCed while clear() is in progress', async function (t) { let db = testCommon.factory() - db.open(function (err) { - t.ifError(err, 'no open error') - - // Insert test data - db.batch(sourceData.slice(), function (err) { - t.ifError(err, 'no batch error') + await db.open() + await db.batch(sourceData.slice()) - // Start async work - db.clear(function () { - t.pass('got callback') + // Start async work + const promise = db.clear() - // Give GC another chance to run, to rule out other issues. - setImmediate(function () { - if (global.gc) global.gc() - t.pass() - }) - }) + // Remove reference. The db should not get garbage collected + // until after the clear() callback, thanks to a napi_ref. + db = null - // Remove reference. The db should not get garbage collected - // until after the clear() callback, thanks to a napi_ref. - db = null + // Useful for manual testing with "node --expose-gc". + // The pending tap assertion may also allow GC to kick in. + if (global.gc) global.gc() - // Useful for manual testing with "node --expose-gc". - // The pending tap assertion may also allow GC to kick in. - if (global.gc) global.gc() - }) - }) + return promise }) diff --git a/test/compact-range-test.js b/test/compact-range-test.js index 7de0fe0c..ee32e2a6 100644 --- a/test/compact-range-test.js +++ b/test/compact-range-test.js @@ -2,70 +2,60 @@ const test = require('tape') const testCommon = require('./common') -const noop = () => {} let db -test('setUp db', function (t) { +test('compactRange() setup', async function (t) { db = testCommon.factory() - db.open(t.end.bind(t)) + return db.open() }) -test('test compactRange() throws if arguments are missing', function (t) { - for (const args of [[], ['foo'], [noop], ['foo', noop]]) { - t.throws(() => db.compactRange(...args), { - name: 'TypeError', - message: "The arguments 'start' and 'end' are required" - }) +test('compactRange() throws if arguments are missing', async function (t) { + t.plan(2 * 2) + + for (const args of [[], ['foo']]) { + try { + await db.compactRange(...args) + } catch (err) { + t.is(err.name, 'TypeError') + t.is(err.message, "The arguments 'start' and 'end' are required") + } } - t.end() }) -test('test compactRange() frees disk space after key deletion', function (t) { +test('compactRange() frees disk space after key deletion', async function (t) { const key1 = '000000' const key2 = '000001' const val1 = Buffer.allocUnsafe(64).fill(1) const val2 = Buffer.allocUnsafe(64).fill(1) - db.batch().put(key1, val1).put(key2, val2).write(function (err) { - t.ifError(err, 'no batch put error') - - db.compactRange(key1, key2, function (err) { - t.ifError(err, 'no compactRange1 error') - - db.approximateSize('0', 'z', function (err, sizeAfterPuts) { - t.error(err, 'no approximateSize1 error') + await db.batch().put(key1, val1).put(key2, val2).write() + await db.compactRange(key1, key2) - db.batch().del(key1).del(key2).write(function (err) { - t.ifError(err, 'no batch del error') + const sizeAfterPuts = await db.approximateSize('0', 'z') - db.compactRange(key1, key2, function (err) { - t.ifError(err, 'no compactRange2 error') + await db.batch().del(key1).del(key2).write() + await db.compactRange(key1, key2) - db.approximateSize('0', 'z', function (err, sizeAfterCompact) { - t.error(err, 'no approximateSize2 error') - t.ok(sizeAfterCompact < sizeAfterPuts) - t.end() - }) - }) - }) - }) - }) - }) + const sizeAfterCompact = await db.approximateSize('0', 'z') + t.ok(sizeAfterCompact < sizeAfterPuts) }) -test('tearDown', function (t) { - db.close(t.end.bind(t)) +test('compactRange() teardown', async function (t) { + return db.close() }) -test('test compactRange() yields error if db is closed', function (t) { - db.compactRange('foo', 'foo', function (err) { - t.is(err && err.code, 'LEVEL_DATABASE_NOT_OPEN') - t.end() - }) +test('compactRange() yields error if db is closed', async function (t) { + t.plan(1) + + try { + await db.compactRange('foo', 'foo') + } catch (err) { + t.is(err.code, 'LEVEL_DATABASE_NOT_OPEN') + } }) -test('test compactRange() is deferred', async function (t) { +test('compactRange() is deferred', async function (t) { const opening = db.open().then(() => 'opening') const deferred = db.compactRange('a', 'b').then(() => 'deferred') t.is(await Promise.race([opening, deferred]), 'opening') @@ -74,7 +64,7 @@ test('test compactRange() is deferred', async function (t) { }) // NOTE: copied from encoding-down -test('encodes start and end of compactRange()', async function (t) { +test('compactRange() encodes start and end', async function (t) { const calls = [] const keyEncoding = { name: 'test', @@ -93,7 +83,7 @@ test('encodes start and end of compactRange()', async function (t) { }) // NOTE: adapted from encoding-down -test('encodes start and end of compactRange() with custom encoding', async function (t) { +test('compactRange() encodes start and end with custom encoding', async function (t) { const calls = [] const keyEncoding = { name: 'test', diff --git a/test/compression-test.js b/test/compression-test.js index da1f73c0..ba38a17f 100644 --- a/test/compression-test.js +++ b/test/compression-test.js @@ -1,8 +1,6 @@ 'use strict' -const each = require('async-each') const du = require('du') -const delayed = require('delayed') const testCommon = require('./common') const { ClassicLevel } = require('..') const test = require('tape') @@ -15,73 +13,82 @@ const multiples = 10 const dataSize = compressableData.length * multiples const verify = function (location, compression, t) { - du(location, function (err, size) { - t.error(err) - if (compression) { - t.ok(size < dataSize, 'on-disk size (' + size + ') is less than data size (' + dataSize + ')') - } else { - t.ok(size >= dataSize, 'on-disk size (' + size + ') is greater than data size (' + dataSize + ')') - } - t.end() + return new Promise(function (resolve, reject) { + du(location, function (err, size) { + if (err) return reject(err) + + if (compression) { + t.ok(size < dataSize, 'on-disk size (' + size + ') is less than data size (' + dataSize + ')') + } else { + t.ok(size >= dataSize, 'on-disk size (' + size + ') is greater than data size (' + dataSize + ')') + } + + resolve() + }) }) } // close, open, close again.. 'compaction' is also performed on open()s -const cycle = function (db, compression, t, callback) { +const cycle = async function (db, compression) { const location = db.location - db.close(function (err) { - t.error(err) - db = new ClassicLevel(location) - db.open({ errorIfExists: false, compression }, function () { - t.error(err) - db.close(function (err) { - t.error(err) - callback() - }) - }) - }) + await db.close() + db = new ClassicLevel(location) + await db.open({ errorIfExists: false, compression }) + return db.close() } test('compression', function (t) { - t.plan(3) + t.plan(4) - t.test('test data is compressed by default (db.put())', function (t) { + t.test('data is compressed by default (db.put())', async function (t) { const db = testCommon.factory() - db.open(function (err) { - t.error(err) - each( - Array.apply(null, Array(multiples)).map(function (e, i) { - return [i, compressableData] - }), function (args, callback) { - db.put.apply(db, args.concat([callback])) - }, cycle.bind(null, db, true, t, delayed.delayed(verify.bind(null, db.location, true, t), 0.01)) - ) + await db.open() + + const promises = Array.apply(null, Array(multiples)).map(function (e, i) { + return db.put(String(i), compressableData) }) + + await Promise.all(promises) + await cycle(db, true) + await verify(db.location, true, t) }) - t.test('test data is not compressed with compression=false on open() (db.put())', function (t) { + t.test('data is not compressed with compression=false on open() (db.put())', async function (t) { const db = testCommon.factory() - db.open({ compression: false }, function (err) { - t.error(err) - each( - Array.apply(null, Array(multiples)).map(function (e, i) { - return [i, compressableData] - }), function (args, callback) { - db.put.apply(db, args.concat([callback])) - }, cycle.bind(null, db, false, t, delayed.delayed(verify.bind(null, db.location, false, t), 0.01)) - ) + await db.open({ compression: false }) + + const promises = Array.apply(null, Array(multiples)).map(function (e, i) { + return db.put(String(i), compressableData) }) + + await Promise.all(promises) + await cycle(db, false) + await verify(db.location, false, t) }) - t.test('test data is compressed by default (db.batch())', function (t) { + t.test('data is compressed by default (db.batch())', async function (t) { const db = testCommon.factory() - db.open(function (err) { - t.error(err) - db.batch( - Array.apply(null, Array(multiples)).map(function (e, i) { - return { type: 'put', key: i, value: compressableData } - }), cycle.bind(null, db, true, t, delayed.delayed(verify.bind(null, db.location, true, t), 0.01)) - ) + await db.open() + + const operations = Array.apply(null, Array(multiples)).map(function (e, i) { + return { type: 'put', key: String(i), value: compressableData } + }) + + await db.batch(operations) + await cycle(db, true) + await verify(db.location, true, t) + }) + + t.test('data is not compressed with compression=false on factory (db.batch())', async function (t) { + const db = testCommon.factory({ compression: false }) + await db.open() + + const operations = Array.apply(null, Array(multiples)).map(function (e, i) { + return { type: 'put', key: String(i), value: compressableData } }) + + await db.batch(operations) + await cycle(db, false) + await verify(db.location, false, t) }) }) diff --git a/test/destroy-test.js b/test/destroy-test.js index eff3413d..7d802951 100644 --- a/test/destroy-test.js +++ b/test/destroy-test.js @@ -3,6 +3,7 @@ const test = require('tape') const tempy = require('tempy') const fs = require('fs') +const fsp = require('fs/promises') const path = require('path') const mkfiletree = require('mkfiletree') const readfiletree = require('readfiletree') @@ -10,20 +11,21 @@ const rimraf = require('rimraf') const { ClassicLevel } = require('..') const makeTest = require('./make') -test('test destroy() without location throws', function (t) { - t.throws(ClassicLevel.destroy, { - name: 'TypeError', - message: "The first argument 'location' must be a non-empty string" - }) - t.throws(() => ClassicLevel.destroy(''), { - name: 'TypeError', - message: "The first argument 'location' must be a non-empty string" - }) - t.end() +test('test destroy() without location throws', async function (t) { + t.plan(2 * 2) + + for (const args of [[], ['']]) { + try { + await ClassicLevel.destroy(...args) + } catch (err) { + t.is(err.name, 'TypeError') + t.is(err.message, "The first argument 'location' must be a non-empty string") + } + } }) test('test destroy non-existent directory', function (t) { - t.plan(4) + t.plan(3) const location = tempy.directory() const parent = path.dirname(location) @@ -32,31 +34,29 @@ test('test destroy non-existent directory', function (t) { t.ok(fs.existsSync(parent), 'parent exists before') // Cleanup to avoid conflicts with other tests + // TODO: use promise rimraf(location, { glob: false }, function (err) { t.ifError(err, 'no error from rimraf()') - ClassicLevel.destroy(location, function (err) { - t.error(err, 'no error') - + ClassicLevel.destroy(location).then(function () { // Assert that destroy() didn't inadvertently create the directory. // Or if it did, that it was at least cleaned up afterwards. t.notOk(fs.existsSync(location), 'directory does not exist after') - }) + }, t.fail.bind(t)) }) }) test('test destroy non-existent parent directory', function (t) { - t.plan(3) + t.plan(2) const location = '/1/2/3/4' const parent = path.dirname(location) t.notOk(fs.existsSync(parent), 'parent does not exist before') - ClassicLevel.destroy(location, function (err) { - t.error(err, 'no error') + ClassicLevel.destroy(location).then(function () { t.notOk(fs.existsSync(location), 'directory does not exist after') - }) + }, t.fail.bind(t)) }) test('test destroy non leveldb directory', function (t) { @@ -65,12 +65,11 @@ test('test destroy non leveldb directory', function (t) { bar: { one: 'ONE', two: 'TWO', three: 'THREE' } } + // TODO: use promise and/or simplify this test mkfiletree.makeTemp('destroy-test', tree, function (err, dir) { t.ifError(err, 'no error from makeTemp()') - ClassicLevel.destroy(dir, function (err) { - t.ifError(err, 'no error from destroy()') - + ClassicLevel.destroy(dir).then(function () { readfiletree(dir, function (err, actual) { t.ifError(err, 'no error from readfiletree()') t.deepEqual(actual, tree, 'directory remains untouched') @@ -80,40 +79,26 @@ test('test destroy non leveldb directory', function (t) { t.end() }) }) - }) + }, t.fail.bind(t)) }) }) -makeTest('test destroy() cleans and removes leveldb-only dir', function (db, t, done) { +makeTest('test destroy() cleans and removes leveldb-only dir', async function (db, t) { const location = db.location - db.close(function (err) { - t.ifError(err, 'no error from close()') - ClassicLevel.destroy(location, function (err) { - t.ifError(err, 'no error from destroy()') - t.notOk(fs.existsSync(location), 'directory completely removed') + await db.close() + await ClassicLevel.destroy(location) - done(null, false) - }) - }) + t.notOk(fs.existsSync(location), 'directory completely removed') }) -makeTest('test destroy() cleans and removes only leveldb parts of a dir', function (db, t, done) { +makeTest('test destroy() cleans and removes only leveldb parts of a dir', async function (db, t) { const location = db.location fs.writeFileSync(path.join(location, 'foo'), 'FOO') - db.close(function (err) { - t.ifError(err, 'no error from close()') + await db.close() + await ClassicLevel.destroy(location) - ClassicLevel.destroy(location, function (err) { - t.ifError(err, 'no error from destroy()') - - readfiletree(location, function (err, tree) { - t.ifError(err, 'no error from readfiletree()') - t.deepEqual(tree, { foo: 'FOO' }, 'non-leveldb files left intact') - - done(null, false) - }) - }) - }) + t.same(await fsp.readdir(location), ['foo'], 'non-leveldb files left intact') + t.same(await fsp.readFile(path.join(location, 'foo'), 'utf8'), 'FOO', 'content left intact') }) diff --git a/test/env-cleanup-hook-test.js b/test/env-cleanup-hook-test.js index cf20143d..ef9eac1f 100644 --- a/test/env-cleanup-hook-test.js +++ b/test/env-cleanup-hook-test.js @@ -18,7 +18,9 @@ function addTest (steps) { test(`cleanup on environment exit (${steps.join(', ')})`, function (t) { t.plan(3) - const child = fork(path.join(__dirname, 'env-cleanup-hook.js'), steps) + const child = fork(path.join(__dirname, 'env-cleanup-hook.js'), steps, { + execArgv: [...process.execArgv, '--unhandled-rejections=strict'] + }) child.on('message', function (m) { t.is(m, steps[steps.length - 1], `got to step: ${m}`) diff --git a/test/env-cleanup-hook.js b/test/env-cleanup-hook.js index ee2d7303..cf26a33e 100644 --- a/test/env-cleanup-hook.js +++ b/test/env-cleanup-hook.js @@ -1,8 +1,9 @@ 'use strict' const testCommon = require('./common') +const noop = () => {} -function test (steps) { +async function test (steps) { let step function nextStep () { @@ -13,7 +14,8 @@ function test (steps) { if (nextStep() !== 'create') { // Send a message triggering an environment exit // and indicating at which step we stopped. - return process.send(step) + process.send(step) + return } const db = testCommon.factory() @@ -21,39 +23,33 @@ function test (steps) { if (nextStep() !== 'open') { if (nextStep() === 'open-error') { // If opening fails the cleanup hook should be a noop. - db.open({ createIfMissing: false, errorIfExists: true }, function (err) { - if (!err) throw new Error('Expected an open() error') - }) + db.open({ createIfMissing: false, errorIfExists: true }).then(function () { + throw new Error('Expected an open() error') + }, noop) } return process.send(step) } // Open the db, expected to be closed by the cleanup hook. - db.open(function (err) { - if (err) throw err - - if (nextStep() === 'create-iterator') { - // Create an iterator, expected to be closed by the cleanup hook. - const it = db.iterator() - - if (nextStep() === 'nexting') { - // This async work should finish before the cleanup hook is called. - it.next(function (err) { - if (err) throw err - }) - } - } + await db.open() + + if (nextStep() === 'create-iterator') { + // Create an iterator, expected to be closed by the cleanup hook. + const it = db.iterator() - if (nextStep() === 'close') { - // Close the db, after which the cleanup hook is a noop. - db.close(function (err) { - if (err) throw err - }) + if (nextStep() === 'nexting') { + // This async work should finish before the cleanup hook is called. + it.next() } + } - process.send(step) - }) + if (nextStep() === 'close') { + // Close the db, after which the cleanup hook is a noop. + db.close() + } + + process.send(step) } test(process.argv.slice(2)) diff --git a/test/getproperty-test.js b/test/getproperty-test.js index 9e7b8e5e..e7e3981f 100644 --- a/test/getproperty-test.js +++ b/test/getproperty-test.js @@ -5,12 +5,12 @@ const testCommon = require('./common') let db -test('setUp db', function (t) { +test('getProperty() setup', async function (t) { db = testCommon.factory() - db.open(t.end.bind(t)) + return db.open() }) -test('test argument-less getProperty() throws', function (t) { +test('argument-less getProperty() throws', function (t) { t.throws(db.getProperty.bind(db), { name: 'TypeError', message: "The first argument 'property' must be a string" @@ -18,7 +18,7 @@ test('test argument-less getProperty() throws', function (t) { t.end() }) -test('test non-string getProperty() throws', function (t) { +test('non-string getProperty() throws', function (t) { t.throws(db.getProperty.bind(db, {}), { name: 'TypeError', message: "The first argument 'property' must be a string" @@ -26,13 +26,13 @@ test('test non-string getProperty() throws', function (t) { t.end() }) -test('test invalid getProperty() returns empty string', function (t) { +test('invalid getProperty() returns empty string', function (t) { t.equal(db.getProperty('foo'), '', 'invalid property') t.equal(db.getProperty('leveldb.foo'), '', 'invalid leveldb.* property') t.end() }) -test('test invalid getProperty("leveldb.num-files-at-levelN") returns numbers', function (t) { +test('invalid getProperty("leveldb.num-files-at-levelN") returns numbers', function (t) { for (let i = 0; i < 7; i++) { t.equal(db.getProperty('leveldb.num-files-at-level' + i), '0', '"leveldb.num-files-at-levelN" === "0"') @@ -40,12 +40,12 @@ test('test invalid getProperty("leveldb.num-files-at-levelN") returns numbers', t.end() }) -test('test invalid getProperty("leveldb.stats")', function (t) { +test('invalid getProperty("leveldb.stats")', function (t) { t.ok(db.getProperty('leveldb.stats').split('\n').length > 3, 'leveldb.stats has > 3 newlines') t.end() }) -test('test invalid getProperty("leveldb.sstables")', function (t) { +test('invalid getProperty("leveldb.sstables")', function (t) { const expected = [0, 1, 2, 3, 4, 5, 6].map(function (l) { return '--- level ' + l + ' ---' }).join('\n') + '\n' @@ -53,8 +53,8 @@ test('test invalid getProperty("leveldb.sstables")', function (t) { t.end() }) -test('tearDown', function (t) { - db.close(t.end.bind(t)) +test('getProperty() teardown', async function (t) { + return db.close() }) test('getProperty() throws if db is closed', function (t) { diff --git a/test/iterator-gc-test.js b/test/iterator-gc-test.js index c7391bf8..0f3bb605 100644 --- a/test/iterator-gc-test.js +++ b/test/iterator-gc-test.js @@ -14,50 +14,37 @@ for (let i = 0; i < 1e3; i++) { // 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', function (t) { - t.plan(6) - +test('db without ref does not get GCed while iterating', async function (t) { let db = testCommon.factory() - db.open(function (err) { - t.ifError(err, 'no open error') - - // Insert test data - db.batch(sourceData.slice(), function (err) { - t.ifError(err, 'no batch error') - - // 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() - iterate(it) - } 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. - setTimeout(iterate.bind(null, it), 1000) - } - }) - }) + await db.open() + + // Insert test data + await db.batch(sourceData.slice()) - function iterate (it) { - // No reference to db here, could be GCed. It shouldn't.. - it.all(function (err, entries) { - t.ifError(err, 'no iterator error') - t.is(entries.length, sourceData.length, 'got data') + // Set highWaterMarkBytes to 0 so that we don't preemptively fetch. + const it = db.iterator({ highWaterMarkBytes: 0 }) - // Because we also have a reference on the iterator. That's the fix. - t.ok(it.db, 'abstract iterator has reference to db') + // Remove reference + db = null - // Which as luck would have it, also allows us to properly end this test. - it.db.close(function (err) { - t.ifError(err, 'no close error') - }) - }) + 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() }) diff --git a/test/iterator-hwm-test.js b/test/iterator-hwm-test.js index 6ca2c464..8571c1d4 100644 --- a/test/iterator-hwm-test.js +++ b/test/iterator-hwm-test.js @@ -7,6 +7,7 @@ let db test('highWaterMarkBytes setup', async function (t) { db = testCommon.factory() + await db.open() // Write 8 bytes return db.batch().put('a', '0').put('b', '1').put('c', '2').put('d', '3').write() diff --git a/test/iterator-recursion-test.js b/test/iterator-recursion-test.js index 79f535f7..c551203e 100644 --- a/test/iterator-recursion-test.js +++ b/test/iterator-recursion-test.js @@ -5,8 +5,6 @@ const testCommon = require('./common') const fork = require('child_process').fork const path = require('path') -let db - const sourceData = (function () { const d = [] let i = 0 @@ -22,65 +20,43 @@ const sourceData = (function () { return d }()) -// TODO: fix this test. It asserted that we didn't segfault if user code had an -// infinite loop leading to stack exhaustion, which caused a node::FatalException() +// NOTE: this is an old leveldown test that asserts that we don't segfault if user code +// has an infinite loop leading to stack exhaustion, which caused a node::FatalException() // call in our Iterator to segfault. This was fixed in 2014 (commit 85e6a38). -// -// Today (2020), we see occasional failures in CI again. We no longer call -// node::FatalException() so there's a new reason. -test.skip('try to create an iterator with a blown stack', function (t) { - for (let i = 0; i < 100; i++) { - t.test(`try to create an iterator with a blown stack (${i})`, function (t) { - t.plan(3) - - // Reducing the stack size down from the default 984 for the child node - // process makes it easier to trigger the bug condition. But making it too low - // causes the child process to die for other reasons. - const opts = { execArgv: ['--stack-size=128'] } - const child = fork(path.join(__dirname, 'stack-blower.js'), ['run'], opts) - - child.on('message', function (m) { - t.ok(true, m) - child.disconnect() - }) - - child.on('exit', function (code, sig) { - t.is(code, 0, 'child exited normally') - t.is(sig, null, 'not terminated due to signal') - }) - }) - } - - t.end() -}) +test('try to create an iterator with a blown stack', function (t) { + t.plan(3) + + // Reducing the stack size down from the default 984 for the child node + // process makes it easier to trigger the bug condition. But making it too low + // causes the child process to die for other reasons. + const opts = { execArgv: ['--stack-size=256'] } + const child = fork(path.join(__dirname, 'stack-blower.js'), ['run'], opts) + + child.on('message', function (m) { + t.ok(true, m) + child.disconnect() + }) -test('setUp db', function (t) { - db = testCommon.factory() - db.open(function (err) { - t.error(err) - db.batch(sourceData, t.end.bind(t)) + child.on('exit', function (code, sig) { + t.is(code, 0, 'child exited normally') + t.is(sig, null, 'not terminated due to signal') }) }) -test('iterate over a large iterator with a large watermark', function (t) { +test('iterate over a large iterator with a large watermark', async function (t) { + const db = testCommon.factory() + + await db.open() + await db.batch(sourceData) + const iterator = db.iterator({ highWaterMarkBytes: 10000000 }) - const read = function () { - iterator.next(function (err, key, value) { - if (err) throw err - if (key === undefined && value === undefined) { - t.end() - } else { - read() - } - }) + while (true) { + const entry = await iterator.next() + if (entry === undefined) break } - read() -}) - -test('tearDown', function (t) { - db.close(t.end.bind(t)) + return db.close() }) diff --git a/test/iterator-starvation-test.js b/test/iterator-starvation-test.js index 89bf4ac3..33d601df 100644 --- a/test/iterator-starvation-test.js +++ b/test/iterator-starvation-test.js @@ -4,9 +4,11 @@ const test = require('tape') const testCommon = require('./common') const sourceData = [] -// For this test the number of records in the db must be a multiple of -// the hardcoded fast-future limit (1000) or a cache size limit in C++. -for (let i = 0; i < 1e4; i++) { +// For this test the number of entries in the db must be a multiple of +// the hardcoded limit in iterator.js (1000). +const limit = 1000 + +for (let i = 0; i < limit * 10; i++) { sourceData.push({ type: 'put', key: i.toString(), @@ -14,107 +16,91 @@ for (let i = 0; i < 1e4; i++) { }) } -test('iterator does not starve event loop', function (t) { - t.plan(6) +test('iterator does not starve event loop', async function (t) { + t.plan(2) const db = testCommon.factory() - db.open(function (err) { - t.ifError(err, 'no open error') - - // Insert test data - db.batch(sourceData.slice(), function (err) { - t.ifError(err, 'no batch error') - - // Set a high highWaterMarkBytes to fill up the cache entirely - const it = db.iterator({ highWaterMarkBytes: Math.pow(1024, 3) }) - - let breaths = 0 - let entries = 0 - let scheduled = false - - // Iterate continuously while also scheduling work with setImmediate(), - // which should be given a chance to run because we limit the tick depth. - const next = function () { - it.next(function (err, key, value) { - if (err || (key === undefined && value === undefined)) { - t.ifError(err, 'no next error') - t.is(entries, sourceData.length, 'got all data') - t.is(breaths, sourceData.length / 1000, 'breathed while iterating') - - return db.close(function (err) { - t.ifError(err, 'no close error') - }) - } - - entries++ - - if (!scheduled) { - scheduled = true - setImmediate(function () { - breaths++ - scheduled = false - }) - } - - next() - }) - } - - next() - }) - }) + // Insert test data + await db.open() + await db.batch(sourceData.slice()) + + // Set a high highWaterMarkBytes to fill up the cache entirely + const it = db.iterator({ highWaterMarkBytes: Math.pow(1024, 3) }) + + let breaths = 0 + let entries = 0 + let scheduled = false + + // Iterate continuously while also scheduling work with setImmediate(), + // which should be given a chance to run because we limit the tick depth. + const next = async function () { + const entry = await it.next() + + if (entry === undefined) { + t.is(entries, sourceData.length, 'got all data') + t.is(breaths, sourceData.length / limit, 'breathed while iterating') + + return db.close() + } + + entries++ + + if (!scheduled) { + scheduled = true + setImmediate(function () { + breaths++ + scheduled = false + }) + } + + return next() + } + + return next() }) -test('iterator with seeks does not starve event loop', function (t) { - t.plan(6) +test('iterator with seeks does not starve event loop', async function (t) { + t.plan(2) const db = testCommon.factory() - db.open(function (err) { - t.ifError(err, 'no open error') + await db.open() + await db.batch(sourceData.slice()) - db.batch(sourceData.slice(), function (err) { - t.ifError(err, 'no batch error') + const it = db.iterator({ highWaterMarkBytes: Math.pow(1024, 3), limit: sourceData.length }) - const it = db.iterator({ highWaterMarkBytes: Math.pow(1024, 3), limit: sourceData.length }) + let breaths = 0 + let entries = 0 + let scheduled = false - let breaths = 0 - let entries = 0 - let scheduled = false + const next = async function () { + const entry = await it.next() - const next = function () { - it.next(function (err, key, value) { - if (err || (key === undefined && value === undefined)) { - t.ifError(err, 'no next error') - t.is(entries, sourceData.length, 'got all data') - t.is(breaths, sourceData.length - 1, 'breathed while iterating') + if (entry === undefined) { + t.is(entries, sourceData.length, 'got all data') + t.is(breaths, sourceData.length - 1, 'breathed while iterating') - return db.close(function (err) { - t.ifError(err, 'no close error') - }) - } + return db.close() + } - entries++ + entries++ - if (!scheduled) { - // Seeking clears the cache, which should only have a positive - // effect because it means the cache must be refilled, which - // again gives us time to breathe. This is a smoke test, really. - it.seek(sourceData[0].key) + if (!scheduled) { + // Seeking clears the cache, which should only have a positive + // effect because it means the cache must be refilled, which + // again gives us time to breathe. This is a smoke test, really. + it.seek(sourceData[0].key) - scheduled = true - setImmediate(function () { - breaths++ - scheduled = false - }) - } + scheduled = true + setImmediate(function () { + breaths++ + scheduled = false + }) + } - next() - }) - } + return next() + } - next() - }) - }) + return next() }) diff --git a/test/iterator-test.js b/test/iterator-test.js index 50706d0a..fe764d3b 100644 --- a/test/iterator-test.js +++ b/test/iterator-test.js @@ -1,41 +1,39 @@ 'use strict' -const make = require('./make') - -make('iterator optimized for seek', function (db, t, done) { - const batch = db.batch() - batch.put('a', 1) - batch.put('b', 1) - batch.put('c', 1) - batch.put('d', 1) - batch.put('e', 1) - batch.put('f', 1) - batch.put('g', 1) - batch.write(function (err) { - const ite = db.iterator() - t.ifError(err, 'no error from batch()') - ite.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key.toString(), 'a', 'key matches') - t.equal(ite.cached, 0, 'no cache') - ite.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key.toString(), 'b', 'key matches') - t.ok(ite.cached > 0, 'has cached items') - ite.seek('d') - t.is(ite.cached, 0, 'cache is emptied') - ite.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key.toString(), 'd', 'key matches') - t.equal(ite.cached, 0, 'no cache') - ite.next(function (err, key, value) { - t.ifError(err, 'no error from next()') - t.equal(key.toString(), 'e', 'key matches') - t.ok(ite.cached > 0, 'has cached items') - ite.close(done) - }) - }) - }) - }) - }) +const test = require('tape') +const testCommon = require('./common') + +test('iterator optimized for seek', async function (t) { + const db = testCommon.factory() + + await db.open() + await db.batch() + .put('a', 'value') + .put('b', 'value') + .put('c', 'value') + .put('d', 'value') + .put('e', 'value') + .put('f', 'value') + .put('g', 'value') + .write() + + const ite = db.iterator() + + t.same(await ite.next(), ['a', 'value'], 'entry matches') + t.is(ite.cached, 0, 'no cache') + + t.same(await ite.next(), ['b', 'value'], 'entry matches') + t.ok(ite.cached > 0, 'has cached items') + + ite.seek('d') + t.is(ite.cached, 0, 'cache is emptied') + + t.same(await ite.next(), ['d', 'value'], 'entry matches') + t.is(ite.cached, 0, 'no cache') + + t.same(await ite.next(), ['e', 'value'], 'entry matches') + t.ok(ite.cached > 0, 'has cached items') + + await ite.close() + return db.close() }) diff --git a/test/leak-tester-batch.js b/test/leak-tester-batch.js index 4f7af335..212f0c3c 100644 --- a/test/leak-tester-batch.js +++ b/test/leak-tester-batch.js @@ -5,30 +5,29 @@ const CHAINED = false const testCommon = require('./common') const crypto = require('crypto') -const assert = require('assert') let writeCount = 0 let rssBase -function print () { - if (writeCount % 100 === 0) { +function tick () { + if (++writeCount % 100 === 0) { if (typeof global.gc !== 'undefined') global.gc() console.log( 'writeCount =', writeCount, ', rss =', Math.round(process.memoryUsage().rss / rssBase * 100) + '%', - Math.round(process.memoryUsage().rss / 1024 / 1024) + 'M', - JSON.stringify([0, 1, 2, 3, 4, 5, 6].map(function (l) { - return db.getProperty('leveldb.num-files-at-level' + l) - })) + Math.round(process.memoryUsage().rss / 1024 / 1024) + 'M' ) } } const run = CHAINED - ? function () { + ? async function () { const batch = db.batch() + // TODO: a good amount of memory usage (and growth) comes from this code and not the db + // itself, which makes the output difficult to interpret. See if we can use fixed data + // without changing the meaning of the test. Same below (for non-chained). for (let i = 0; i < 100; i++) { let key = 'long key to test memory usage ' + String(Math.floor(Math.random() * 10000000)) if (BUFFERS) key = Buffer.from(key) @@ -37,15 +36,10 @@ const run = CHAINED batch.put(key, value) } - batch.write(function (err) { - assert(!err) - process.nextTick(run) - }) - - writeCount++ - print() + tick() + return batch.write() } - : function () { + : async function () { const batch = [] for (let i = 0; i < 100; i++) { @@ -56,18 +50,13 @@ const run = CHAINED batch.push({ type: 'put', key, value }) } - db.batch(batch, function (err) { - assert(!err) - process.nextTick(run) - }) - - writeCount++ - print() + tick() + return db.batch(batch) } const db = testCommon.factory() -db.open(function () { +db.open().then(async function () { rssBase = process.memoryUsage().rss - run() + while (true) await run() }) diff --git a/test/leak-tester-iterator.js b/test/leak-tester-iterator.js index 50ca019c..5d0a6020 100644 --- a/test/leak-tester-iterator.js +++ b/test/leak-tester-iterator.js @@ -9,39 +9,36 @@ if (!global.gc) { console.error('To force GC, run with "node --expose-gc"') } -function run () { - const it = db.iterator() +async function run () { + while (true) { + const it = db.iterator() - it.next(function (err) { - if (err) throw err + await it.next() + await it.close() - it.close(function (err) { - if (err) throw err + if (!rssBase) { + rssBase = process.memoryUsage().rss + } - if (!rssBase) { - rssBase = process.memoryUsage().rss - } + if (++count % 1000 === 0) { + if (global.gc) global.gc() - if (++count % 1000 === 0) { - if (global.gc) global.gc() + const rss = process.memoryUsage().rss + const percent = Math.round((rss / rssBase) * 100) + const mb = Math.round(rss / 1024 / 1024) - const rss = process.memoryUsage().rss - const percent = Math.round((rss / rssBase) * 100) - const mb = Math.round(rss / 1024 / 1024) - - console.log('count = %d, rss = %d% %dM', count, percent, mb) - } - - run() - }) - }) + console.log('count = %d, rss = %d% %dM', count, percent, mb) + } + } } -db.open(function (err) { - if (err) throw err +async function main () { + await db.open() + await db.put('key', 'value') + await run() +} - db.put('key', 'value', function (err) { - if (err) throw err - run() - }) +main().catch(function (err) { + console.error(err) + process.exit(1) }) diff --git a/test/leak-tester.js b/test/leak-tester.js index 1308d016..7974e737 100644 --- a/test/leak-tester.js +++ b/test/leak-tester.js @@ -7,43 +7,45 @@ const crypto = require('crypto') let putCount = 0 let getCount = 0 -let rssBase +let iterations = 0 -function run () { - let key = 'long key to test memory usage ' + String(Math.floor(Math.random() * 10000000)) +async function main () { + const db = testCommon.factory() + await db.open() - if (BUFFERS) key = Buffer.from(key) + const rssBase = process.memoryUsage().rss - db.get(key, function (err, value) { - getCount++ + while (true) { + let testKey = 'long key to test memory usage ' + String(Math.floor(Math.random() * 10000000)) + let testValue = crypto.randomBytes(1024) - if (err) { - let putValue = crypto.randomBytes(1024) - if (!BUFFERS) putValue = putValue.toString('hex') + if (BUFFERS) { + testKey = Buffer.from(testKey, 'utf8') + } else { + testValue = testValue.toString('hex') + } + + const value = await db.get(testKey, { fillCache: false }) - return db.put(key, putValue, function () { - putCount++ - process.nextTick(run) - }) + if (value === undefined) { + await db.put(testKey, testValue) + putCount++ + } else { + getCount++ } - process.nextTick(run) - }) - - if (getCount % 1000 === 0) { - if (typeof global.gc !== 'undefined') global.gc() - console.log('getCount =', getCount, ', putCount = ', putCount, ', rss =', - Math.round(process.memoryUsage().rss / rssBase * 100) + '%', - Math.round(process.memoryUsage().rss / 1024 / 1024) + 'M', - JSON.stringify([0, 1, 2, 3, 4, 5, 6].map(function (l) { - return db.getProperty('leveldb.num-files-at-level' + l) - }))) + if (iterations++ % 5e3 === 0) { + if (typeof global.gc !== 'undefined') global.gc() + + console.log('getCount =', getCount, ', putCount = ', putCount, ', rss =', + Math.round(process.memoryUsage().rss / rssBase * 100) + '%', + Math.round(process.memoryUsage().rss / 1024 / 1024) + 'M' + ) + } } } -const db = testCommon.factory() - -db.open(function () { - rssBase = process.memoryUsage().rss - run() +main().catch(function (err) { + console.error(err) + process.exit(1) }) diff --git a/test/lock-test.js b/test/lock-test.js index cbc0aeb8..6f7d6324 100644 --- a/test/lock-test.js +++ b/test/lock-test.js @@ -4,6 +4,7 @@ const test = require('tape') const tempy = require('tempy') const fork = require('child_process').fork const path = require('path') +const { once } = require('events') const { ClassicLevel } = require('..') test('lock held by same process', async function (t) { @@ -24,29 +25,26 @@ test('lock held by same process', async function (t) { return db1.close() }) -test('lock held by other process', function (t) { - t.plan(6) +test('lock held by other process', async function (t) { + t.plan(4) const location = tempy.directory() const db = new ClassicLevel(location) + await db.open() - db.open(function (err) { - t.ifError(err, 'no open error') + const child = fork(path.join(__dirname, 'lock.js'), [location]) - const child = fork(path.join(__dirname, 'lock.js'), [location]) + child.on('message', function (err) { + t.is(err.code, 'LEVEL_DATABASE_NOT_OPEN', 'second process failed to open') + t.is(err.cause.code, 'LEVEL_LOCKED', 'second process got lock error') - child.on('message', function (err) { - t.is(err.code, 'LEVEL_DATABASE_NOT_OPEN', 'second process failed to open') - t.is(err.cause.code, 'LEVEL_LOCKED', 'second process got lock error') + child.disconnect() + }) - child.disconnect() - }) + const [code, sig] = await once(child, 'exit') - child.on('exit', function (code, sig) { - t.is(code, 0, 'child exited normally') - t.is(sig, null, 'not terminated due to signal') + t.is(code, 0, 'child exited normally') + t.is(sig, null, 'not terminated due to signal') - db.close(t.ifError.bind(t)) - }) - }) + return db.close() }) diff --git a/test/lock.js b/test/lock.js index d74f6acb..bd5b89ef 100644 --- a/test/lock.js +++ b/test/lock.js @@ -5,6 +5,8 @@ const { ClassicLevel } = require('..') const location = process.argv[2] const db = new ClassicLevel(location) -db.open(function (err) { +db.open().then(function () { + process.send(null) +}, function (err) { process.send(err) }) diff --git a/test/make.js b/test/make.js index 2d8ac356..0a34267a 100644 --- a/test/make.js +++ b/test/make.js @@ -4,32 +4,17 @@ const test = require('tape') const testCommon = require('./common') function makeTest (name, testFn) { - test(name, function (t) { + test(name, async function (t) { const db = testCommon.factory() - const done = function (err, close) { - t.ifError(err, 'no error from done()') - if (close === false) { - t.end() - return - } + await db.open() + await db.batch([ + { type: 'put', key: 'a', value: '1' }, + { type: 'put', key: 'b', value: '2' }, + { type: 'put', key: 'c', value: '3' } + ]) - db.close(function (err) { - t.ifError(err, 'no error from close()') - t.end() - }) - } - db.open(function (err) { - t.ifError(err, 'no error from open()') - db.batch([ - { type: 'put', key: 'one', value: '1' }, - { type: 'put', key: 'two', value: '2' }, - { type: 'put', key: 'three', value: '3' } - ], function (err) { - t.ifError(err, 'no error from batch()') - testFn(db, t, done) - }) - }) + return testFn(db, t) }) } diff --git a/test/repair-test.js b/test/repair-test.js index 68b892c8..4e91b3c8 100644 --- a/test/repair-test.js +++ b/test/repair-test.js @@ -1,52 +1,48 @@ 'use strict' const test = require('tape') -const fs = require('fs') +const fsp = require('fs/promises') const { ClassicLevel } = require('..') const makeTest = require('./make') -test('test repair() without location throws', function (t) { - t.throws(ClassicLevel.repair, { - name: 'TypeError', - message: "The first argument 'location' must be a non-empty string" - }) - t.throws(() => ClassicLevel.repair(''), { - name: 'TypeError', - message: "The first argument 'location' must be a non-empty string" - }) - t.end() +test('test repair() without location throws', async function (t) { + t.plan(2 * 2) + + for (const args of [[], ['']]) { + try { + await ClassicLevel.repair(...args) + } catch (err) { + t.is(err.name, 'TypeError') + t.is(err.message, "The first argument 'location' must be a non-empty string") + } + } }) -test('test repair non-existent directory returns error', function (t) { - ClassicLevel.repair('/1/2/3/4', function (err) { +test('test repair non-existent directory returns error', async function (t) { + t.plan(1) + + try { + await ClassicLevel.repair('/1/2/3/4') + } catch (err) { if (process.platform !== 'win32') { - t.ok(/no such file or directory/i.test(err), 'error on callback') + t.ok(/no such file or directory/i.test(err), 'error') } else { - t.ok(/IO error/i.test(err), 'error on callback') + t.ok(/IO error/i.test(err), 'error') } - t.end() - }) + } }) // a proxy indicator that RepairDB is being called and doing its thing -makeTest('test repair() compacts', function (db, t, done) { - const location = db.location - - db.close(function (err) { - t.ifError(err, 'no error from close()') - - let files = fs.readdirSync(location) - t.ok(files.some(function (f) { return (/\.log$/).test(f) }), 'directory contains log file(s)') - t.notOk(files.some(function (f) { return (/\.ldb$/).test(f) }), 'directory does not contain ldb file(s)') +makeTest('test repair() compacts', async function (db, t) { + await db.close() - ClassicLevel.repair(location, function (err) { - t.ifError(err, 'no error from repair()') + let files = await fsp.readdir(db.location) + t.ok(files.some(function (f) { return (/\.log$/).test(f) }), 'directory contains log file(s)') + t.notOk(files.some(function (f) { return (/\.ldb$/).test(f) }), 'directory does not contain ldb file(s)') - files = fs.readdirSync(location) - t.notOk(files.some(function (f) { return (/\.log$/).test(f) }), 'directory does not contain log file(s)') - t.ok(files.some(function (f) { return (/\.ldb$/).test(f) }), 'directory contains ldb file(s)') + await ClassicLevel.repair(db.location) - done(null, false) - }) - }) + files = await fsp.readdir(db.location) + t.notOk(files.some(function (f) { return (/\.log$/).test(f) }), 'directory does not contain log file(s)') + t.ok(files.some(function (f) { return (/\.ldb$/).test(f) }), 'directory contains ldb file(s)') }) diff --git a/test/segfault-test.js b/test/segfault-test.js index dce70a4d..15041192 100644 --- a/test/segfault-test.js +++ b/test/segfault-test.js @@ -7,83 +7,73 @@ const operations = [] // The db must wait for pending operations to finish before closing. This to // prevent segfaults and in the case of compactRange() to prevent hanging. See // https://github.com/Level/leveldown/issues/157 and 32. -function testPending (name, expectedCount, fn) { +function testPending (name, fn) { operations.push(fn) - test(`close() waits for pending ${name}`, function (t) { + test(`close() waits for pending ${name}`, async function (t) { const db = testCommon.factory() - let count = 0 + let finished = false - db.open(function (err) { - t.ifError(err, 'no error from open()') + await db.open() + await db.put('key', 'value') - db.put('key', 'value', function (err) { - t.ifError(err, 'no error from put()') - - fn(db, function (err) { - count++ - t.ifError(err, 'no error from operation') - }) + fn(db).then(function () { + finished = true + }) - db.close(function (err) { - t.ifError(err, 'no error from close()') - t.is(count, expectedCount, 'operation(s) finished before close') - t.end() - }) - }) + return db.close().then(function () { + t.is(finished, true, 'operation(s) finished before close') }) }) } -testPending('get()', 1, function (db, next) { - db.get('key', next) +testPending('get()', async function (db) { + return db.get('key') }) -testPending('put()', 1, function (db, next) { - db.put('key2', 'value', next) +testPending('put()', async function (db) { + return db.put('key2', 'value') }) -testPending('put() with { sync }', 1, function (db, next) { +testPending('put() with { sync }', async function (db) { // The sync option makes the operation slower and thus more likely to // cause a segfault (if closing were to happen during the operation). - db.put('key2', 'value', { sync: true }, next) + return db.put('key2', 'value', { sync: true }) }) -testPending('del()', 1, function (db, next) { - db.del('key', next) +testPending('del()', async function (db) { + return db.del('key') }) -testPending('del() with { sync }', 1, function (db, next) { - db.del('key', { sync: true }, next) +testPending('del() with { sync }', async function (db) { + return db.del('key', { sync: true }) }) -testPending('batch([])', 1, function (db, next) { - db.batch([{ type: 'del', key: 'key' }], next) +testPending('batch([])', async function (db) { + return db.batch([{ type: 'del', key: 'key' }]) }) -testPending('batch([]) with { sync }', 1, function (db, next) { - db.batch([{ type: 'del', key: 'key' }], { sync: true }, next) +testPending('batch([]) with { sync }', async function (db) { + return db.batch([{ type: 'del', key: 'key' }], { sync: true }) }) -testPending('batch()', 1, function (db, next) { - db.batch().del('key').write(next) +testPending('batch()', async function (db) { + return db.batch().del('key').write() }) -testPending('batch() with { sync }', 1, function (db, next) { - db.batch().del('key').write({ sync: true }, next) +testPending('batch() with { sync }', async function (db) { + return db.batch().del('key').write({ sync: true }) }) -testPending('approximateSize()', 1, function (db, next) { - db.approximateSize('a', 'z', next) +testPending('approximateSize()', async function (db) { + return db.approximateSize('a', 'z') }) -testPending('compactRange()', 1, function (db, next) { - db.compactRange('a', 'z', next) +testPending('compactRange()', async function (db) { + return db.compactRange('a', 'z') }) // Test multiple pending operations, using all of the above. -testPending('operations', operations.length, function (db, next) { - for (const fn of operations.slice(0, -1)) { - fn(db, next) - } +testPending('operations', async function (db) { + return Promise.all(operations.slice(0, -1).map(fn => fn(db))) }) diff --git a/test/stack-blower.js b/test/stack-blower.js index f1ff486b..3e851810 100644 --- a/test/stack-blower.js +++ b/test/stack-blower.js @@ -4,7 +4,7 @@ * This test uses infinite recursion to test iterator creation with limited * stack space. In order to isolate the test harness, we run in a different * process. This is achieved through a fork() command in - * iterator-recursion-test.js. To prevent tap from trying to run this test + * iterator-recursion-test.js. To prevent tape from trying to run this test * directly, we check for a command-line argument. */ const testCommon = require('./common') @@ -13,17 +13,20 @@ if (process.argv[2] === 'run') { const db = testCommon.factory() let depth = 0 - db.open(function () { - function recurse () { - db.iterator({ gte: '0' }) - depth++ - recurse() - } + db.open().then(function () { + // Escape promise chain + process.nextTick(function () { + function recurse () { + db.iterator({ gte: '0' }) + depth++ + recurse() + } - try { - recurse() - } catch (e) { - process.send('Catchable error at depth ' + depth) - } + try { + recurse() + } catch (e) { + process.send('Catchable error at depth ' + depth) + } + }) }) }