Skip to content

Commit d150b8a

Browse files
authored
fix memory leak in allMarshal (#12)
Changes the implementation of `allMarshal` to follow closely the evolution of the implementation of `all`. Adds a script that can be run as `npm run test:memory` to check for obvious memory leaks. Prior to this fix, a typical run looked like: ``` 0 8081154048 1000 7443259392 2000 6777753600 3000 6128287744 4000 5500166144 5000 4849360896 6000 4221939712 7000 3558346752 8000 2960449536 9000 2309222400 10000 1655738368 11000 1019441152 ``` After this fix, a typical run looks like: ``` 0 8291295232 1000 8268148736 2000 8252329984 3000 8253804544 4000 8252588032 5000 8261718016 6000 8248782848 7000 8268709888 8000 8249749504 9000 8258523136 10000 8242008064 11000 8255168512 ```
1 parent 208d600 commit d150b8a

File tree

3 files changed

+39
-5
lines changed

3 files changed

+39
-5
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"name": "@gristlabs/sqlite3",
33
"description": "Asynchronous, non-blocking SQLite3 bindings",
4-
"version": "5.1.4-grist.7",
4+
"version": "5.1.4-grist.8",
55
"homepage": "https://github.com/gristlabs/node-sqlite3",
66
"author": {
77
"name": "Mapbox",
@@ -74,7 +74,7 @@
7474
"install": "node-pre-gyp install --fallback-to-build",
7575
"pretest": "node test/support/createdb.js",
7676
"test": "mocha -R spec --timeout 480000",
77-
"rebuild-tests": "node-gyp rebuild --directory test/cpp",
77+
"test:memory": "node test/support/memory_check.js",
7878
"pack": "node-pre-gyp package"
7979
},
8080
"license": "BSD-3-Clause",

src/statement.cc

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ void Statement::Work_BeginAllMarshal(Baton* baton) {
637637
void Statement::Work_AllMarshal(napi_env e, void* data) {
638638
STATEMENT_INIT(MarshalBaton);
639639

640-
sqlite3_mutex* mtx = sqlite3_db_mutex(stmt->db->_handle);
640+
STATEMENT_MUTEX(mtx);
641641
sqlite3_mutex_enter(mtx);
642642

643643
sqlite3_stmt* sqstmt = stmt->_handle;
@@ -701,11 +701,13 @@ void Statement::Work_AllMarshal(napi_env e, void* data) {
701701
}
702702

703703
void Statement::Work_AfterAllMarshal(napi_env e, napi_status status, void* data) {
704-
STATEMENT_INIT(MarshalBaton);
704+
std::unique_ptr<MarshalBaton> baton(static_cast<MarshalBaton*>(data));
705+
Statement* stmt = baton->stmt;
705706

706707
Napi::Env env = stmt->Env();
708+
Napi::HandleScope scope(env);
707709
if (stmt->status != SQLITE_DONE) {
708-
Error(baton);
710+
Error(baton.get());
709711
} else {
710712
// Fire callbacks.
711713
Napi::Function cb = baton->callback.Value();

test/support/memory_check.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* A quick test for memory leakage.
3+
* Run as "npm run test:memory".
4+
* Reports a loop count and a free memory number. If the free memory
5+
* number drops precipitously, uh-oh. Memory reported is for full system,
6+
* so don't do too much in background while this runs.
7+
*/
8+
9+
const sqlite3 = require('../..');
10+
const util = require('util');
11+
const os = require('os');
12+
13+
async function run() {
14+
const db = new sqlite3.Database(':memory:');
15+
const run = util.promisify(db.run.bind(db));
16+
const fetch = util.promisify(db.allMarshal.bind(db));
17+
console.log('Creating table');
18+
await run('CREATE TABLE foo (row text, blb blob)');
19+
for (let i = 0; i < 10000; i++) {
20+
await run('INSERT INTO foo VALUES(?, ?)',
21+
'row ' + i, 'g\x00\x00\xc0\xff\xff\xff\xdfA');
22+
}
23+
console.log('Running queries');
24+
for (let i = 0; i < 100000; i++) {
25+
if (i % 1000 === 0) {
26+
console.log(String(i).padStart(20), String(os.freemem()).padStart(20));
27+
}
28+
await fetch('SELECT * FROM foo');
29+
}
30+
}
31+
32+
run().catch(e => console.log(e));

0 commit comments

Comments
 (0)