From 9e5e42c2ff6aa7e654866e1d932d1329573ee25e Mon Sep 17 00:00:00 2001 From: Benjamin DROUARD Date: Wed, 9 Oct 2019 16:35:29 +0200 Subject: [PATCH 1/7] Using NAN_MODULE_WORKER_ENABLED to support multiple workers --- src/node_sqlite3.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_sqlite3.cc b/src/node_sqlite3.cc index 10f88ea68..90631dc97 100644 --- a/src/node_sqlite3.cc +++ b/src/node_sqlite3.cc @@ -108,4 +108,4 @@ const char* sqlite_authorizer_string(int type) { } } -NODE_MODULE(node_sqlite3, RegisterModule) +NAN_MODULE_WORKER_ENABLED(node_sqlite3, RegisterModule) From 21a69c860cd1d702c60ce2aba6b2310851fddb57 Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Sat, 23 Jun 2018 15:49:20 -0400 Subject: [PATCH 2/7] Add support for node 10.5's --experimental-worker threading support --- src/async.h | 4 ++-- src/database.cc | 16 ++++++++-------- src/database.h | 6 +++++- src/macros.h | 2 +- src/statement.cc | 2 +- src/statement.h | 2 +- 6 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/async.h b/src/async.h index 5232c127d..29ca28a0f 100644 --- a/src/async.h +++ b/src/async.h @@ -22,11 +22,11 @@ template class Async { Parent* parent; public: - Async(Parent* parent_, Callback cb_) + Async(uv_loop_t* loop_, Parent* parent_, Callback cb_) : callback(cb_), parent(parent_) { watcher.data = this; NODE_SQLITE3_MUTEX_INIT - uv_async_init(uv_default_loop(), &watcher, reinterpret_cast(listener)); + uv_async_init(loop_, &watcher, reinterpret_cast(listener)); } static void listener(uv_async_t* handle, int status) { diff --git a/src/database.cc b/src/database.cc index ace5fa0b0..b6ffcf1d4 100644 --- a/src/database.cc +++ b/src/database.cc @@ -127,7 +127,7 @@ NAN_METHOD(Database::New) { callback = Local::Cast(info[pos++]); } - Database* db = new Database(); + Database* db = new Database(node::GetCurrentEventLoop(info.GetIsolate())); db->Wrap(info.This()); Nan::ForceSet(info.This(), Nan::New("filename").ToLocalChecked(), info[0].As(), ReadOnly); @@ -141,7 +141,7 @@ NAN_METHOD(Database::New) { } void Database::Work_BeginOpen(Baton* baton) { - int status = uv_queue_work(uv_default_loop(), + int status = uv_queue_work(baton->db->loop, &baton->request, Work_Open, (uv_after_work_cb)Work_AfterOpen); assert(status == 0); } @@ -227,7 +227,7 @@ void Database::Work_BeginClose(Baton* baton) { baton->db->RemoveCallbacks(); baton->db->closing = true; - int status = uv_queue_work(uv_default_loop(), + int status = uv_queue_work(baton->db->loop, &baton->request, Work_Close, (uv_after_work_cb)Work_AfterClose); assert(status == 0); } @@ -391,7 +391,7 @@ void Database::RegisterTraceCallback(Baton* baton) { if (db->debug_trace == NULL) { // Add it. - db->debug_trace = new AsyncTrace(db, TraceCallback); + db->debug_trace = new AsyncTrace(db->loop, db, TraceCallback); sqlite3_trace(db->_handle, TraceCallback, db); } else { @@ -429,7 +429,7 @@ void Database::RegisterProfileCallback(Baton* baton) { if (db->debug_profile == NULL) { // Add it. - db->debug_profile = new AsyncProfile(db, ProfileCallback); + db->debug_profile = new AsyncProfile(db->loop, db, ProfileCallback); sqlite3_profile(db->_handle, ProfileCallback, db); } else { @@ -470,7 +470,7 @@ void Database::RegisterUpdateCallback(Baton* baton) { if (db->update_event == NULL) { // Add it. - db->update_event = new AsyncUpdate(db, UpdateCallback); + db->update_event = new AsyncUpdate(db->loop, db, UpdateCallback); sqlite3_update_hook(db->_handle, UpdateCallback, db); } else { @@ -525,7 +525,7 @@ void Database::Work_BeginExec(Baton* baton) { assert(baton->db->open); assert(baton->db->_handle); assert(baton->db->pending == 0); - int status = uv_queue_work(uv_default_loop(), + int status = uv_queue_work(baton->db->loop, &baton->request, Work_Exec, (uv_after_work_cb)Work_AfterExec); assert(status == 0); } @@ -625,7 +625,7 @@ void Database::Work_BeginLoadExtension(Baton* baton) { assert(baton->db->open); assert(baton->db->_handle); assert(baton->db->pending == 0); - int status = uv_queue_work(uv_default_loop(), + int status = uv_queue_work(baton->db->loop, &baton->request, Work_LoadExtension, reinterpret_cast(Work_AfterLoadExtension)); assert(status == 0); } diff --git a/src/database.h b/src/database.h index 1ec101548..10093dd9c 100644 --- a/src/database.h +++ b/src/database.h @@ -101,8 +101,9 @@ class Database : public Nan::ObjectWrap { friend class Backup; protected: - Database() : Nan::ObjectWrap(), + Database(uv_loop_t* loop_) : Nan::ObjectWrap(), _handle(NULL), + loop(loop_), open(false), closing(false), locked(false), @@ -173,7 +174,10 @@ class Database : public Nan::ObjectWrap { protected: sqlite3* _handle; +public: + uv_loop_t* loop; +protected: bool open; bool closing; bool locked; diff --git a/src/macros.h b/src/macros.h index 9c0136cc5..87e5e4300 100644 --- a/src/macros.h +++ b/src/macros.h @@ -125,7 +125,7 @@ const char* sqlite_authorizer_string(int type); assert(baton->stmt->prepared); \ baton->stmt->locked = true; \ baton->stmt->db->pending++; \ - int status = uv_queue_work(uv_default_loop(), \ + int status = uv_queue_work(baton->stmt->db->loop, \ &baton->request, \ Work_##type, reinterpret_cast(Work_After##type)); \ assert(status == 0); diff --git a/src/statement.cc b/src/statement.cc index e09aeafff..6ca8b0a33 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -115,7 +115,7 @@ NAN_METHOD(Statement::New) { void Statement::Work_BeginPrepare(Database::Baton* baton) { assert(baton->db->open); baton->db->pending++; - int status = uv_queue_work(uv_default_loop(), + int status = uv_queue_work(baton->db->loop, &baton->request, Work_Prepare, (uv_after_work_cb)Work_AfterPrepare); assert(status == 0); } diff --git a/src/statement.h b/src/statement.h index 90d295b70..39f12d3c0 100644 --- a/src/statement.h +++ b/src/statement.h @@ -174,7 +174,7 @@ class Statement : public Nan::ObjectWrap { watcher.data = this; NODE_SQLITE3_MUTEX_INIT stmt->Ref(); - uv_async_init(uv_default_loop(), &watcher, async_cb); + uv_async_init(stmt->db->loop, &watcher, async_cb); } ~Async() { From e556ba3da5b5f3d87652867db624208e127f55e5 Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Sat, 23 Jun 2018 15:55:04 -0400 Subject: [PATCH 3/7] Add test:worker script that runs the mocha tests inside a worker --- package.json | 1 + scripts/mocha-as-worker.js | 13 +++++++++++++ 2 files changed, 14 insertions(+) create mode 100644 scripts/mocha-as-worker.js diff --git a/package.json b/package.json index 530f3c709..d49a928c1 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "pretest": "node test/support/createdb.js", "test": "mocha -R spec --timeout 480000", "pack": "node-pre-gyp package" + "test:worker": "node --experimental-worker scripts/mocha-as-worker.js -R spec --timeout 480000" }, "license": "BSD-3-Clause", "keywords": [ diff --git a/scripts/mocha-as-worker.js b/scripts/mocha-as-worker.js new file mode 100644 index 000000000..9803e89de --- /dev/null +++ b/scripts/mocha-as-worker.js @@ -0,0 +1,13 @@ +// Run the mocha tests in a worker +// Not a clean approach, but is sufficient to verify correctness +const worker_threads = require("worker_threads"); +const path = require("path"); + +if (worker_threads.isMainThread) { + const worker = new worker_threads.Worker(__filename, { workerData: { windowSize: process.stdout.getWindowSize() } }); + worker.on("error", console.error); +} else { + process.stdout.getWindowSize = () => worker_threads.workerData.windowSize; + const mochaPath = path.resolve(require.resolve("mocha"), "../bin/_mocha"); + require(mochaPath); +} From e88a531ce708200d1c4356af607af45e29051ccf Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Sat, 23 Jun 2018 16:10:20 -0400 Subject: [PATCH 4/7] Use the default loop when running on node 9 or earlier --- src/database.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/database.cc b/src/database.cc index b6ffcf1d4..6c7cd60da 100644 --- a/src/database.cc +++ b/src/database.cc @@ -127,7 +127,12 @@ NAN_METHOD(Database::New) { callback = Local::Cast(info[pos++]); } - Database* db = new Database(node::GetCurrentEventLoop(info.GetIsolate())); +#if NODE_MODULE_VERSION > NODE_9_0_MODULE_VERSION + uv_loop_t* loop = node::GetCurrentEventLoop(info.GetIsolate()); +#else + uv_loop_t* loop = uv_default_loop(); +#endif + Database* db = new Database(loop); db->Wrap(info.This()); Nan::ForceSet(info.This(), Nan::New("filename").ToLocalChecked(), info[0].As(), ReadOnly); From d4b1aa23d35b9fbcc86559f19a437119b29d59e5 Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Mon, 25 Jun 2018 00:02:03 -0400 Subject: [PATCH 5/7] Store unique constructor templates per-thread so that the constructor validation works correctly from multiple contexts --- src/database.cc | 2 +- src/database.h | 2 +- src/statement.cc | 2 +- src/statement.h | 2 +- src/threading.h | 15 +++++++++++++++ 5 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/database.cc b/src/database.cc index 6c7cd60da..fde3e8099 100644 --- a/src/database.cc +++ b/src/database.cc @@ -6,7 +6,7 @@ using namespace node_sqlite3; -Nan::Persistent Database::constructor_template; +NODE_SQLITE3_THREAD_LOCAL Nan::Persistent Database::constructor_template; NAN_MODULE_INIT(Database::Init) { Nan::HandleScope scope; diff --git a/src/database.h b/src/database.h index 10093dd9c..5bf1ce145 100644 --- a/src/database.h +++ b/src/database.h @@ -20,7 +20,7 @@ class Database; class Database : public Nan::ObjectWrap { public: - static Nan::Persistent constructor_template; + NODE_SQLITE3_THREAD_LOCAL static Nan::Persistent constructor_template; static NAN_MODULE_INIT(Init); static inline bool HasInstance(Local val) { diff --git a/src/statement.cc b/src/statement.cc index 6ca8b0a33..88e9c6c5f 100644 --- a/src/statement.cc +++ b/src/statement.cc @@ -9,7 +9,7 @@ using namespace node_sqlite3; -Nan::Persistent Statement::constructor_template; +NODE_SQLITE3_THREAD_LOCAL Nan::Persistent Statement::constructor_template; NAN_MODULE_INIT(Statement::Init) { Nan::HandleScope scope; diff --git a/src/statement.h b/src/statement.h index 39f12d3c0..a82e79c1e 100644 --- a/src/statement.h +++ b/src/statement.h @@ -73,7 +73,7 @@ typedef Row Parameters; class Statement : public Nan::ObjectWrap { public: - static Nan::Persistent constructor_template; + NODE_SQLITE3_THREAD_LOCAL static Nan::Persistent constructor_template; static NAN_MODULE_INIT(Init); static NAN_METHOD(New); diff --git a/src/threading.h b/src/threading.h index fe738a4c0..d465787a2 100644 --- a/src/threading.h +++ b/src/threading.h @@ -45,4 +45,19 @@ #endif +#if __cplusplus >= 201103L + + #define NODE_SQLITE3_THREAD_LOCAL thread_local + +#elif defined(_WIN32) + + #define NODE_SQLITE3_THREAD_LOCAL __declspec(thread) + +#else + + #define NODE_SQLITE3_THREAD_LOCAL __thread + +#endif + + #endif // NODE_SQLITE3_SRC_THREADING_H From 7f184aba9a9f0f2731c8032641f9cf1f03b83e20 Mon Sep 17 00:00:00 2001 From: Ryan Petrich Date: Mon, 25 Jun 2018 00:03:52 -0400 Subject: [PATCH 6/7] Force loading sqlite3 module in both the main thead and background workers inside test:workers (to ensure that it's possible to use in multiple contexts and not just the context where node-sqlite3 was first loaded) --- scripts/mocha-as-worker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/mocha-as-worker.js b/scripts/mocha-as-worker.js index 9803e89de..81e29c109 100644 --- a/scripts/mocha-as-worker.js +++ b/scripts/mocha-as-worker.js @@ -4,6 +4,7 @@ const worker_threads = require("worker_threads"); const path = require("path"); if (worker_threads.isMainThread) { + require(".."); const worker = new worker_threads.Worker(__filename, { workerData: { windowSize: process.stdout.getWindowSize() } }); worker.on("error", console.error); } else { From cb87051ad56890d203283f25589a9fecd70abe25 Mon Sep 17 00:00:00 2001 From: Benjamin DROUARD Date: Wed, 9 Oct 2019 17:14:16 +0200 Subject: [PATCH 7/7] Fix package.json --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index d49a928c1..20623ebb8 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "install": "node-pre-gyp install --fallback-to-build", "pretest": "node test/support/createdb.js", "test": "mocha -R spec --timeout 480000", - "pack": "node-pre-gyp package" + "pack": "node-pre-gyp package", "test:worker": "node --experimental-worker scripts/mocha-as-worker.js -R spec --timeout 480000" }, "license": "BSD-3-Clause",