Skip to content

Commit 43d8fdf

Browse files
committed
sqlite: run backup completion in callback scope
Run SQLite backup after-work callbacks inside an internal callback scope. This drains promise reactions and next ticks before the event loop can become idle. Add a child-process regression test where backup is the final active request. Assisted-by: Codex Signed-off-by: Filip Skokan <panva.ip@gmail.com>
1 parent 045ff95 commit 43d8fdf

3 files changed

Lines changed: 56 additions & 3 deletions

File tree

src/node_sqlite.cc

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -624,9 +624,13 @@ class BackupJob : public ThreadPoolWork {
624624
}
625625

626626
void AfterThreadPoolWork(int status) override {
627-
HandleScope handle_scope(env()->isolate());
627+
Isolate* isolate = env()->isolate();
628+
HandleScope handle_scope(isolate);
629+
Context::Scope context_scope(env()->context());
630+
InternalCallbackScope callback_scope(
631+
env(), Object::New(isolate), {0, 0}, InternalCallbackScope::kNoFlags);
628632
Local<Promise::Resolver> resolver =
629-
Local<Promise::Resolver>::New(env()->isolate(), resolver_);
633+
Local<Promise::Resolver>::New(isolate, resolver_);
630634

631635
if (!(backup_status_ == SQLITE_OK || backup_status_ == SQLITE_DONE ||
632636
backup_status_ == SQLITE_BUSY || backup_status_ == SQLITE_LOCKED)) {
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import { backup, DatabaseSync } from 'node:sqlite';
2+
3+
const source = new DatabaseSync(':memory:');
4+
source.exec(`
5+
CREATE TABLE data(value);
6+
INSERT INTO data VALUES (zeroblob(1048576));
7+
`);
8+
9+
let keepAlive = setInterval(() => {}, 1_000);
10+
let settled = false;
11+
12+
process.once('beforeExit', () => {
13+
if (!settled) {
14+
process.stderr.write('backup promise did not settle before the event loop became idle\n');
15+
process.exit(1);
16+
}
17+
});
18+
19+
backup(source, process.argv[2], {
20+
rate: 1,
21+
progress() {
22+
if (keepAlive !== undefined) {
23+
clearInterval(keepAlive);
24+
keepAlive = undefined;
25+
}
26+
},
27+
}).then(() => {
28+
settled = true;
29+
source.close();
30+
}, (error) => {
31+
settled = true;
32+
process.stderr.write(`${error.stack ?? error}\n`);
33+
process.exitCode = 1;
34+
});

test/parallel/test-sqlite-backup.mjs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
// Flags: --expose-gc
2-
import { isWindows, skipIfSQLiteMissing } from '../common/index.mjs';
2+
import {
3+
isWindows,
4+
skipIfSQLiteMissing,
5+
spawnPromisified,
6+
} from '../common/index.mjs';
7+
import fixtures from '../common/fixtures.js';
38
import tmpdir from '../common/tmpdir.js';
49
import { join } from 'node:path';
510
import { describe, test } from 'node:test';
@@ -364,3 +369,13 @@ test('source database is kept alive while a backup is in flight', async (t) => {
364369
const rows = backupDb.prepare('SELECT COUNT(*) AS n FROM data').get();
365370
t.assert.strictEqual(rows.n, 500);
366371
});
372+
373+
test('backup promise settles when the backup is the last active request', async (t) => {
374+
const { code, signal, stderr } = await spawnPromisified(process.execPath, [
375+
fixtures.path('sqlite', 'backup-last-request.mjs'),
376+
nextDb(),
377+
]);
378+
379+
t.assert.strictEqual(signal, null);
380+
t.assert.strictEqual(code, 0, stderr);
381+
});

0 commit comments

Comments
 (0)