Skip to content

Commit c284189

Browse files
penbergpenberg
authored
Fix Statement.run() locking the database (#190)
2 parents 95c7793 + 23d12f2 commit c284189

File tree

5 files changed

+222
-5
lines changed

5 files changed

+222
-5
lines changed

‎integration-tests/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"type": "module",
44
"private": true,
55
"scripts": {
6-
"test": "cross-env PROVIDER=sqlite ava tests/sync.test.js && cross-env LIBSQL_JS_DEV=1 PROVIDER=libsql ava tests/sync.test.js && cross-env LIBSQL_JS_DEV=1 ava tests/async.test.js && cross-env LIBSQL_JS_DEV=1 ava tests/extensions.test.js"
6+
"test": "cross-env PROVIDER=sqlite ava tests/sync.test.js && cross-env LIBSQL_JS_DEV=1 PROVIDER=libsql ava tests/sync.test.js && cross-env LIBSQL_JS_DEV=1 ava tests/async.test.js && cross-env LIBSQL_JS_DEV=1 ava tests/extensions.test.js ava tests/concurrency.test.js"
77
},
88
"devDependencies": {
99
"ava": "^5.3.0",

‎integration-tests/tests/async.test.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ test.serial("Statement.run() [positional]", async (t) => {
5050
const info = stmt.run(["Carol", "carol@example.net"]);
5151
t.is(info.changes, 1);
5252
t.is(info.lastInsertRowid, 3);
53+
54+
// Verify that the data is inserted
55+
const stmt2 = await db.prepare("SELECT * FROM users WHERE id = 3");
56+
t.is(stmt2.get().name, "Carol");
57+
t.is(stmt2.get().email, "carol@example.net");
5358
});
5459

5560
test.serial("Statement.get() returns no rows", async (t) => {
@@ -315,7 +320,7 @@ test.serial("errors", async (t) => {
315320

316321
test.serial("Database.prepare() after close()", async (t) => {
317322
const db = t.context.db;
318-
await db.close();
323+
db.close();
319324
await t.throwsAsync(async () => {
320325
await db.prepare("SELECT 1");
321326
}, {
@@ -326,7 +331,7 @@ test.serial("Database.prepare() after close()", async (t) => {
326331

327332
test.serial("Database.exec() after close()", async (t) => {
328333
const db = t.context.db;
329-
await db.close();
334+
db.close();
330335
await t.throwsAsync(async () => {
331336
await db.exec("SELECT 1");
332337
}, {

‎integration-tests/tests/concurrency.test.js‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
import test from "ava";
2+
import crypto from 'crypto';
3+
import fs from 'fs';
4+
5+
test.beforeEach(async (t) => {
6+
const [db, errorType, path] = await connect();
7+
8+
await db.exec(`
9+
DROP TABLE IF EXISTS users;
10+
CREATE TABLE users (id TEXT PRIMARY KEY, name TEXT, email TEXT)
11+
`);
12+
const aliceId = generateUUID();
13+
const bobId = generateUUID();
14+
await db.exec(
15+
`INSERT INTO users (id, name, email) VALUES ('${aliceId}', 'Alice', 'alice@example.org')`
16+
);
17+
await db.exec(
18+
`INSERT INTO users (id, name, email) VALUES ('${bobId}', 'Bob', 'bob@example.com')`
19+
);
20+
t.context = {
21+
db,
22+
errorType,
23+
aliceId,
24+
bobId,
25+
path
26+
};
27+
});
28+
29+
test("Concurrent reads", async (t) => {
30+
const db = t.context.db;
31+
const stmt = await db.prepare("SELECT * FROM users WHERE id = ?");
32+
33+
const promises = [];
34+
for (let i = 0; i < 100; i++) {
35+
promises.push(stmt.get(t.context.aliceId));
36+
promises.push(stmt.get(t.context.bobId));
37+
}
38+
39+
const results = await Promise.all(promises);
40+
41+
for (let i = 0; i < results.length; i++) {
42+
const result = results[i];
43+
t.truthy(result);
44+
t.is(typeof result.name, 'string');
45+
t.is(typeof result.email, 'string');
46+
}
47+
cleanup(t.context);
48+
});
49+
50+
test("Concurrent writes", async (t) => {
51+
const db = t.context.db;
52+
53+
await db.exec(`
54+
DROP TABLE IF EXISTS concurrent_users;
55+
CREATE TABLE concurrent_users (
56+
id TEXT PRIMARY KEY,
57+
name TEXT,
58+
email TEXT
59+
)
60+
`);
61+
62+
const stmt = await db.prepare("INSERT INTO concurrent_users(id, name, email) VALUES (:id, :name, :email)");
63+
64+
const promises = [];
65+
for (let i = 0; i < 50; i++) {
66+
promises.push(stmt.run({
67+
id: generateUUID(),
68+
name: `User${i}`,
69+
email: `user${i}@example.com`
70+
}));
71+
}
72+
73+
await Promise.all(promises);
74+
75+
const countStmt = await db.prepare("SELECT COUNT(*) as count FROM concurrent_users");
76+
const result = await countStmt.get();
77+
t.is(result.count, 50);
78+
79+
cleanup(t.context);
80+
});
81+
82+
test("Concurrent transaction isolation", async (t) => {
83+
const db = t.context.db;
84+
85+
await db.exec(`
86+
DROP TABLE IF EXISTS transaction_users;
87+
CREATE TABLE transaction_users (
88+
id TEXT PRIMARY KEY,
89+
name TEXT,
90+
email TEXT
91+
)
92+
`);
93+
94+
const aliceId = generateUUID();
95+
const bobId = generateUUID();
96+
97+
await db.exec(`
98+
INSERT INTO transaction_users (id, name, email) VALUES
99+
('${aliceId}', 'Alice', 'alice@example.org'),
100+
('${bobId}', 'Bob', 'bob@example.com')
101+
`);
102+
103+
const updateUser = db.transaction(async (id, name, email) => {
104+
const stmt = await db.prepare("UPDATE transaction_users SET name = :name, email = :email WHERE id = :id");
105+
await stmt.run({ id, name, email });
106+
});
107+
108+
const promises = [];
109+
for (let i = 0; i < 10; i++) {
110+
promises.push(updateUser(aliceId, `Alice${i}`, `alice${i}@example.org`));
111+
promises.push(updateUser(bobId, `Bob${i}`, `bob${i}@example.com`));
112+
}
113+
114+
await Promise.all(promises);
115+
116+
const stmt = await db.prepare("SELECT * FROM transaction_users ORDER BY name");
117+
const results = await stmt.all();
118+
t.is(results.length, 2);
119+
t.truthy(results[0].name.startsWith('Alice'));
120+
t.truthy(results[1].name.startsWith('Bob'));
121+
122+
cleanup(t.context);
123+
});
124+
125+
test("Concurrent reads and writes", async (t) => {
126+
const db = t.context.db;
127+
128+
await db.exec(`
129+
DROP TABLE IF EXISTS mixed_users;
130+
CREATE TABLE mixed_users (
131+
id TEXT PRIMARY KEY,
132+
name TEXT,
133+
email TEXT
134+
)
135+
`);
136+
137+
const aliceId = generateUUID();
138+
await db.exec(`
139+
INSERT INTO mixed_users (id, name, email) VALUES
140+
('${aliceId}', 'Alice', 'alice@example.org')
141+
`);
142+
143+
const readStmt = await db.prepare("SELECT * FROM mixed_users WHERE id = ?");
144+
const writeStmt = await db.prepare("INSERT INTO mixed_users(id, name, email) VALUES (:id, :name, :email)");
145+
146+
const promises = [];
147+
for (let i = 0; i < 20; i++) {
148+
promises.push(readStmt.get(aliceId));
149+
writeStmt.run({
150+
id: generateUUID(),
151+
name: `User${i}`,
152+
email: `user${i}@example.com`
153+
});
154+
}
155+
await Promise.all(promises);
156+
157+
const countStmt = await db.prepare("SELECT COUNT(*) as count FROM mixed_users");
158+
const result = await countStmt.get();
159+
t.is(result.count, 21); // 1 initial + 20 new records
160+
161+
await cleanup(t.context);
162+
});
163+
164+
test("Concurrent operations with timeout should handle busy database", async (t) => {
165+
const timeout = 1000;
166+
const path = `test-${crypto.randomBytes(8).toString('hex')}.db`;
167+
const [conn1] = await connect(path);
168+
const [conn2] = await connect(path, { timeout });
169+
170+
await conn1.exec("CREATE TABLE t(id TEXT PRIMARY KEY, x INTEGER)");
171+
await conn1.exec("BEGIN IMMEDIATE");
172+
await conn1.exec(`INSERT INTO t VALUES ('${generateUUID()}', 1)`);
173+
174+
const start = Date.now();
175+
try {
176+
await conn2.exec(`INSERT INTO t VALUES ('${generateUUID()}', 2)`);
177+
t.fail("Should have thrown SQLITE_BUSY error");
178+
} catch (e) {
179+
t.is(e.code, "SQLITE_BUSY");
180+
const end = Date.now();
181+
const elapsed = end - start;
182+
t.true(elapsed > timeout / 2, "Timeout should be respected");
183+
}
184+
185+
conn1.close();
186+
conn2.close();
187+
// FIXME: Fails on Windows because file is still busy.
188+
// fs.unlinkSync(path);
189+
});
190+
191+
192+
const connect = async (path_opt, options = {}) => {
193+
const path = path_opt ?? `test-${crypto.randomBytes(8).toString('hex')}.db`;
194+
const x = await import("libsql/promise");
195+
const db = new x.default(process.env.LIBSQL_DATABASE ?? path, options);
196+
return [db, x.SqliteError, path];
197+
};
198+
199+
const cleanup = async (context) => {
200+
context.db.close();
201+
// FIXME: Fails on Windows because file is still busy.
202+
// fs.unlinkSync(context.path);
203+
};
204+
205+
const generateUUID = () => {
206+
return crypto.randomUUID();
207+
};

‎integration-tests/tests/sync.test.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,11 @@ test.serial("Statement.run() [positional]", async (t) => {
5858
const info = stmt.run(["Carol", "carol@example.net"]);
5959
t.is(info.changes, 1);
6060
t.is(info.lastInsertRowid, 3);
61+
62+
// Verify that the data is inserted
63+
const stmt2 = db.prepare("SELECT * FROM users WHERE id = 3");
64+
t.is(stmt2.get().name, "Carol");
65+
t.is(stmt2.get().email, "carol@example.net");
6166
});
6267

6368
test.serial("Statement.run() [named]", async (t) => {

‎src/lib.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -654,16 +654,16 @@ impl Statement {
654654
let start = std::time::Instant::now();
655655

656656
let mut stmt = self.stmt.lock().await;
657-
stmt.reset();
658657
let params = map_params(&stmt, params)?;
659-
stmt.query(params).await.map_err(Error::from)?;
658+
stmt.run(params).await.map_err(Error::from)?;
660659
let changes = if conn.total_changes() == total_changes_before {
661660
0
662661
} else {
663662
conn.changes()
664663
};
665664
let last_insert_row_id = conn.last_insert_rowid();
666665
let duration = start.elapsed().as_secs_f64();
666+
stmt.reset();
667667
Ok(RunResult {
668668
changes: changes as f64,
669669
duration,

0 commit comments

Comments
 (0)