Skip to content

Commit 47ee71b

Browse files
committed
sqlite: move time out to openconfiguration
1 parent d518370 commit 47ee71b

File tree

5 files changed

+102
-16
lines changed

5 files changed

+102
-16
lines changed

src/node_sqlite.cc

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -461,15 +461,13 @@ DatabaseSync::DatabaseSync(Environment* env,
461461
Local<Object> object,
462462
DatabaseOpenConfiguration&& open_config,
463463
bool open,
464-
bool allow_load_extension,
465-
int timeout)
464+
bool allow_load_extension)
466465
: BaseObject(env, object), open_config_(std::move(open_config)) {
467466
MakeWeak();
468467
connection_ = nullptr;
469468
allow_load_extension_ = allow_load_extension;
470469
enable_load_extension_ = allow_load_extension;
471470
ignore_next_sqlite_error_ = false;
472-
timeout_ = timeout;
473471

474472
if (open) {
475473
Open();
@@ -547,7 +545,7 @@ bool DatabaseSync::Open() {
547545
CHECK_ERROR_OR_THROW(env()->isolate(), this, r, SQLITE_OK, false);
548546
CHECK_EQ(foreign_keys_enabled, open_config_.get_enable_foreign_keys());
549547

550-
sqlite3_busy_timeout(connection_, timeout_);
548+
sqlite3_busy_timeout(connection_, open_config_.get_timeout());
551549

552550
if (allow_load_extension_) {
553551
if (env()->permission()->enabled()) [[unlikely]] {
@@ -667,7 +665,6 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
667665
DatabaseOpenConfiguration open_config(std::move(location.value()));
668666
bool open = true;
669667
bool allow_load_extension = false;
670-
int timeout = 0;
671668
if (args.Length() > 1) {
672669
if (!args[1]->IsObject()) {
673670
THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
@@ -771,19 +768,16 @@ void DatabaseSync::New(const FunctionCallbackInfo<Value>& args) {
771768
if (!timeout_v->IsInt32()) {
772769
THROW_ERR_INVALID_ARG_TYPE(
773770
env->isolate(),
774-
"The \"options.timeout\" argument must be a number.");
771+
"The \"options.timeout\" argument must be an integer.");
775772
return;
776773
}
777-
timeout = timeout_v.As<Int32>()->Value();
774+
775+
open_config.set_timeout(timeout_v.As<Int32>()->Value());
778776
}
779777
}
780778

781-
new DatabaseSync(env,
782-
args.This(),
783-
std::move(open_config),
784-
open,
785-
allow_load_extension,
786-
timeout);
779+
new DatabaseSync(
780+
env, args.This(), std::move(open_config), open, allow_load_extension);
787781
}
788782

789783
void DatabaseSync::Open(const FunctionCallbackInfo<Value>& args) {

src/node_sqlite.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,16 @@ class DatabaseOpenConfiguration {
3535

3636
inline void set_enable_dqs(bool flag) { enable_dqs_ = flag; }
3737

38+
inline void set_timeout(int timeout) { timeout_ = timeout; }
39+
40+
inline int get_timeout() { return timeout_; }
41+
3842
private:
3943
std::string location_;
4044
bool read_only_ = false;
4145
bool enable_foreign_keys_ = true;
4246
bool enable_dqs_ = false;
47+
int timeout_ = 0;
4348
};
4449

4550
class StatementSync;
@@ -51,8 +56,7 @@ class DatabaseSync : public BaseObject {
5156
v8::Local<v8::Object> object,
5257
DatabaseOpenConfiguration&& open_config,
5358
bool open,
54-
bool allow_load_extension,
55-
int timeout);
59+
bool allow_load_extension);
5660
void MemoryInfo(MemoryTracker* tracker) const override;
5761
static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
5862
static void Open(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -92,7 +96,6 @@ class DatabaseSync : public BaseObject {
9296
DatabaseOpenConfiguration open_config_;
9397
bool allow_load_extension_;
9498
bool enable_load_extension_;
95-
int timeout_;
9699
sqlite3* connection_;
97100
bool ignore_next_sqlite_error_;
98101

test/fixtures/sqlite/lock-db.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
const { DatabaseSync } = require('node:sqlite');
2+
3+
const dbPath = process.argv[2];
4+
const conn = new DatabaseSync(dbPath);
5+
6+
conn.exec('BEGIN EXCLUSIVE TRANSACTION');
7+
process.send('locked');
8+
setTimeout(() => {
9+
conn.exec('ROLLBACK');
10+
conn.close();
11+
process.exit(0);
12+
}, 1000);

test/parallel/test-sqlite-database-sync.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,15 @@ suite('DatabaseSync() constructor', () => {
7777
});
7878
});
7979

80+
test('throws if options.timeout is provided but is not a number', (t) => {
81+
t.assert.throws(() => {
82+
new DatabaseSync('foo', { timeout: null });
83+
}, {
84+
code: 'ERR_INVALID_ARG_TYPE',
85+
message: /The "options\.timeout" argument must be a number/,
86+
});
87+
});
88+
8089
test('is not read-only by default', (t) => {
8190
const dbPath = nextDb();
8291
const db = new DatabaseSync(dbPath);

test/parallel/test-sqlite-timeout.js

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
'use strict';
2+
require('../common');
3+
const tmpdir = require('../common/tmpdir');
4+
const { path: fixturesPath } = require('../common/fixtures');
5+
const { spawn } = require('node:child_process');
6+
const { join } = require('node:path');
7+
const { DatabaseSync } = require('node:sqlite');
8+
const { test } = require('node:test');
9+
let cnt = 0;
10+
11+
tmpdir.refresh();
12+
13+
function nextDb() {
14+
return join(tmpdir.path, `database-${cnt++}.db`);
15+
}
16+
17+
test('connection can perform queries when lock is released before the timeout', (t, done) => {
18+
const databasePath = nextDb();
19+
const conn = new DatabaseSync(databasePath, { timeout: 1100 });
20+
t.after(() => { conn.close(); });
21+
22+
conn.exec('CREATE TABLE data (key INTEGER PRIMARY KEY, value TEXT)');
23+
24+
// Spawns a child process to locks the database for 1 second
25+
const child = spawn('./node', [fixturesPath('sqlite/lock-db.js'), databasePath], {
26+
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
27+
});
28+
29+
child.on('message', (msg) => {
30+
if (msg === 'locked') {
31+
conn.exec('INSERT INTO data (key, value) VALUES (?, ?)', [1, 'value-1']);
32+
33+
setTimeout(() => {
34+
done();
35+
}, 1100);
36+
}
37+
});
38+
});
39+
40+
test('trhows if lock is holden longer than the provided timeout', (t, done) => {
41+
const databasePath = nextDb();
42+
const conn = new DatabaseSync(databasePath, { timeout: 800 });
43+
t.after(() => { conn.close(); });
44+
45+
conn.exec('CREATE TABLE data (key INTEGER PRIMARY KEY, value TEXT)');
46+
47+
// Spawns a child process to locks the database for 1 second
48+
const child = spawn('./node', [fixturesPath('sqlite/lock-db.js'), databasePath], {
49+
stdio: ['inherit', 'inherit', 'inherit', 'ipc'],
50+
});
51+
52+
child.on('message', (msg) => {
53+
if (msg === 'locked') {
54+
t.assert.throws(() => {
55+
conn.exec('INSERT INTO data (key, value) VALUES (?, ?)', [1, 'value-1']);
56+
}, {
57+
code: 'ERR_SQLITE_ERROR',
58+
message: 'database is locked',
59+
});
60+
}
61+
});
62+
63+
child.on('exit', (code) => {
64+
if (code === 0) {
65+
done();
66+
}
67+
});
68+
});

0 commit comments

Comments
 (0)