Skip to content

Commit 28ba2bd

Browse files
committed
src: handle race condition
1 parent cada892 commit 28ba2bd

File tree

3 files changed

+60
-0
lines changed

3 files changed

+60
-0
lines changed

src/node_sqlite.cc

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,10 @@ class SQLiteAsyncTask : public ThreadPoolWork {
459459
}
460460

461461
void DoThreadPoolWork() override {
462+
if (db_->IsClosing()) {
463+
cancelled_ = true;
464+
return;
465+
}
462466
if (work_) {
463467
result_ = work_();
464468
}
@@ -470,6 +474,16 @@ class SQLiteAsyncTask : public ThreadPoolWork {
470474
Local<Promise::Resolver> resolver =
471475
Local<Promise::Resolver>::New(isolate, resolver_);
472476

477+
if (cancelled_ || db_->IsClosing()) {
478+
resolver
479+
->Reject(env_->context(),
480+
ERR_INVALID_STATE(isolate, "database is closing"))
481+
.FromJust();
482+
Finalize();
483+
db_->ProcessNextAsyncTask();
484+
return;
485+
}
486+
473487
if (after_) {
474488
after_(result_, resolver);
475489
Finalize();
@@ -486,6 +500,7 @@ class SQLiteAsyncTask : public ThreadPoolWork {
486500
std::function<T()> work_ = nullptr;
487501
std::function<void(T, Local<Promise::Resolver>)> after_ = nullptr;
488502
T result_;
503+
bool cancelled_ = false;
489504
};
490505

491506
// TODO(geeksilva97): Replace BackupJob usage with SQLiteAsyncTask
@@ -1204,6 +1219,7 @@ void Database::Close(const FunctionCallbackInfo<Value>& args) {
12041219
ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
12051220
Environment* env = Environment::GetCurrent(args);
12061221
THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
1222+
db->is_closing_.store(true, std::memory_order_release);
12071223
db->FinalizeStatements();
12081224
db->DeleteSessions();
12091225
int r = sqlite3_close_v2(db->connection_);

src/node_sqlite.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "threadpoolwork-inl.h"
1111
#include "util.h"
1212

13+
#include <atomic>
1314
#include <list>
1415
#include <map>
1516
#include <queue>
@@ -200,6 +201,7 @@ class Database : public BaseObject {
200201
void FinalizeBackups();
201202
void UntrackStatement(Statement* statement);
202203
bool IsOpen();
204+
bool IsClosing() const { return is_closing_.load(std::memory_order_acquire); }
203205
bool is_async() { return open_config_.get_async(); }
204206
bool use_big_ints() const { return open_config_.get_use_big_ints(); }
205207
bool return_arrays() const { return open_config_.get_return_arrays(); }
@@ -231,6 +233,7 @@ class Database : public BaseObject {
231233
bool enable_load_extension_;
232234
sqlite3* connection_;
233235
bool ignore_next_sqlite_error_;
236+
std::atomic<bool> is_closing_{false};
234237

235238
std::set<ThreadPoolWork*> async_tasks_;
236239
std::queue<ThreadPoolWork*> task_queue_;

test/parallel/test-sqlite-database-async.mjs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,3 +562,44 @@ suite('Async mode restrictions', () => {
562562
});
563563
});
564564
});
565+
566+
suite('Database close behavior', () => {
567+
test('rejects pending operations when database is closed', async (t) => {
568+
const db = new Database(':memory:');
569+
await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)');
570+
571+
// Start an async operation but don't await it yet
572+
const pendingOp = db.exec('INSERT INTO test (value) VALUES (\'test\')');
573+
574+
// Close the database immediately
575+
db.close();
576+
577+
// The pending operation should be rejected
578+
await t.assert.rejects(pendingOp, {
579+
code: 'ERR_INVALID_STATE',
580+
message: /database is closing/,
581+
});
582+
});
583+
584+
test('rejects multiple pending operations when database is closed', async (t) => {
585+
const db = new Database(':memory:');
586+
await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)');
587+
588+
// Queue multiple operations
589+
const ops = [
590+
db.exec('INSERT INTO test (value) VALUES (\'test1\')'),
591+
db.exec('INSERT INTO test (value) VALUES (\'test2\')'),
592+
db.exec('INSERT INTO test (value) VALUES (\'test3\')'),
593+
];
594+
595+
// Close the database
596+
db.close();
597+
598+
// All operations should be rejected
599+
const results = await Promise.allSettled(ops);
600+
for (const result of results) {
601+
t.assert.strictEqual(result.status, 'rejected');
602+
t.assert.strictEqual(result.reason.code, 'ERR_INVALID_STATE');
603+
}
604+
});
605+
});

0 commit comments

Comments
 (0)