Skip to content

Commit 45baba7

Browse files
committed
sqlite: test async ordering
1 parent 28ba2bd commit 45baba7

File tree

3 files changed

+82
-14
lines changed

3 files changed

+82
-14
lines changed

src/node_sqlite.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ class SQLiteAsyncTask : public ThreadPoolWork {
481481
.FromJust();
482482
Finalize();
483483
db_->ProcessNextAsyncTask();
484+
db_->MaybeCloseConnection();
484485
return;
485486
}
486487

@@ -780,6 +781,15 @@ void Database::ProcessNextAsyncTask() {
780781
}
781782
}
782783

784+
void Database::MaybeCloseConnection() {
785+
// Close the connection if Close() was called while async tasks were running.
786+
// This is called after the last task completes.
787+
if (IsClosing() && !has_running_task_ && connection_ != nullptr) {
788+
sqlite3_close_v2(connection_);
789+
connection_ = nullptr;
790+
}
791+
}
792+
783793
void Database::DeleteSessions() {
784794
// all attached sessions need to be deleted before the database is closed
785795
// https://www.sqlite.org/session/sqlite3session_create.html
@@ -1222,6 +1232,13 @@ void Database::Close(const FunctionCallbackInfo<Value>& args) {
12221232
db->is_closing_.store(true, std::memory_order_release);
12231233
db->FinalizeStatements();
12241234
db->DeleteSessions();
1235+
1236+
// If there are running async tasks, defer closing the connection until
1237+
// they complete to avoid use-after-free in the thread pool.
1238+
if (db->has_running_task_) {
1239+
return;
1240+
}
1241+
12251242
int r = sqlite3_close_v2(db->connection_);
12261243
CHECK_ERROR_OR_THROW(env->isolate(), db, r, SQLITE_OK, void());
12271244
db->connection_ = nullptr;

src/node_sqlite.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,7 @@ class Database : public BaseObject {
198198
void RemoveAsyncTask(ThreadPoolWork* async_task);
199199
void ScheduleAsyncTask(ThreadPoolWork* async_task);
200200
void ProcessNextAsyncTask();
201+
void MaybeCloseConnection();
201202
void FinalizeBackups();
202203
void UntrackStatement(Statement* statement);
203204
bool IsOpen();

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

Lines changed: 64 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -126,10 +126,10 @@ suite('Database() constructor', () => {
126126
`);
127127
t.after(() => { db.close(); });
128128
await t.assert.rejects(db.exec('INSERT INTO bar (foo_id) VALUES (1)'),
129-
{
130-
code: 'ERR_SQLITE_ERROR',
131-
message: 'FOREIGN KEY constraint failed',
132-
});
129+
{
130+
code: 'ERR_SQLITE_ERROR',
131+
message: 'FOREIGN KEY constraint failed',
132+
});
133133
});
134134

135135
test('allows disabling foreign key constraints', async (t) => {
@@ -563,18 +563,72 @@ suite('Async mode restrictions', () => {
563563
});
564564
});
565565

566-
suite('Database close behavior', () => {
566+
suite('Async operation ordering', () => {
567+
test('executes operations sequentially per database', async (t) => {
568+
const db = new Database(':memory:');
569+
t.after(() => { db.close(); });
570+
await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, seq INTEGER)');
571+
572+
// Launch multiple operations concurrently
573+
const ops = [];
574+
for (let i = 0; i < 10; i++) {
575+
ops.push(db.exec(`INSERT INTO test (seq) VALUES (${i})`));
576+
}
577+
578+
await Promise.all(ops);
579+
580+
// Check they were inserted in order (sequential execution)
581+
const stmt = db.prepare('SELECT id, seq FROM test ORDER BY id');
582+
const rows = await stmt.all();
583+
584+
// Verify sequential: id should match seq + 1 (autoincrement starts at 1)
585+
t.assert.strictEqual(rows.length, 10);
586+
for (const row of rows) {
587+
t.assert.strictEqual(row.id, row.seq + 1);
588+
}
589+
});
590+
591+
test('different connections can execute in parallel', async (t) => {
592+
const db1 = new Database(':memory:');
593+
const db2 = new Database(':memory:');
594+
t.after(() => { db1.close(); db2.close(); });
595+
const times = {};
596+
const now = () => process.hrtime.bigint();
597+
const LONG_QUERY = `
598+
WITH RECURSIVE cnt(x) AS (
599+
SELECT 1
600+
UNION ALL
601+
SELECT x + 1 FROM cnt WHERE x < 300000
602+
)
603+
SELECT sum(x) FROM cnt;
604+
`;
605+
606+
const op = async (db, label) => {
607+
times[label] = { start: now() };
608+
609+
await db.exec(LONG_QUERY);
610+
611+
times[label].end = now();
612+
};
613+
614+
// Start both operations
615+
await Promise.all([op(db1, 'db1'), op(db2, 'db2')]);
616+
617+
// Verify that their execution times overlap
618+
t.assert.ok(
619+
times.db1.start < times.db2.end &&
620+
times.db2.start < times.db1.end
621+
);
622+
});
623+
});
624+
625+
suite('Database.prototype.close', () => {
567626
test('rejects pending operations when database is closed', async (t) => {
568627
const db = new Database(':memory:');
569628
await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)');
570-
571-
// Start an async operation but don't await it yet
572629
const pendingOp = db.exec('INSERT INTO test (value) VALUES (\'test\')');
573-
574-
// Close the database immediately
575630
db.close();
576631

577-
// The pending operation should be rejected
578632
await t.assert.rejects(pendingOp, {
579633
code: 'ERR_INVALID_STATE',
580634
message: /database is closing/,
@@ -584,18 +638,14 @@ suite('Database close behavior', () => {
584638
test('rejects multiple pending operations when database is closed', async (t) => {
585639
const db = new Database(':memory:');
586640
await db.exec('CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)');
587-
588-
// Queue multiple operations
589641
const ops = [
590642
db.exec('INSERT INTO test (value) VALUES (\'test1\')'),
591643
db.exec('INSERT INTO test (value) VALUES (\'test2\')'),
592644
db.exec('INSERT INTO test (value) VALUES (\'test3\')'),
593645
];
594646

595-
// Close the database
596647
db.close();
597648

598-
// All operations should be rejected
599649
const results = await Promise.allSettled(ops);
600650
for (const result of results) {
601651
t.assert.strictEqual(result.status, 'rejected');

0 commit comments

Comments
 (0)