Support calling close() on cached.Database()

Introduce a refcount for cached Database entries so that calling
close() on the Database returned from cached.Database() does not
invalidate existing or future uses of the same cached Database.

This is a backwards-incompatible change because it changes the number
of times close() must be called before the underlying Database is
closed.

Author: rhansen

lib/sqlite3.js

@@ -34,20 +34,32 @@ sqlite3.cached = {
 
         var db;
         file = path.resolve(file);
-        function cb() { callback.call(db, null); }
 
-        if (!sqlite3.cached.objects[file]) {
-            db = sqlite3.cached.objects[file] = new Database(file, a, b);
+        let cacheEntry = sqlite3.cached.objects[file];
+        if (!cacheEntry) {
+            cacheEntry = sqlite3.cached.objects[file] = {refCount: 0};
+            db = cacheEntry.db = new (class extends Database {
+                close(cb) {
+                    if (--cacheEntry.refCount <= 0) {
+                        delete sqlite3.cached.objects[file];
+                        super.close(cb);
+                    } else if (typeof cb === 'function') {
+                        process.nextTick(() => { cb.call(this, null); });
+                    }
+                }
+            })(file, a, b);
         }
         else {
             // Make sure the callback is called.
-            db = sqlite3.cached.objects[file];
+            db = sqlite3.cached.objects[file].db;
             var callback = (typeof a === 'number') ? b : a;
             if (typeof callback === 'function') {
+                const cb = () => { callback.call(db, null); };
                 if (db.open) process.nextTick(cb);
                 else db.once('open', cb);
             }
         }
+        cacheEntry.refCount++;
 
         return db;
     },

package.json

@@ -1,7 +1,7 @@
 {
   "name": "sqlite3",
   "description": "Asynchronous, non-blocking SQLite3 bindings",
-  "version": "5.0.2",
+  "version": "6.0.0",
   "homepage": "https://github.com/mapbox/node-sqlite3",
   "author": {
     "name": "MapBox",

test/cache.test.js

@@ -1,42 +1,133 @@
 var sqlite3 = require('..');
 var assert = require('assert');
 var helper = require('./support/helper');
+var util = require('util');
 
 describe('cache', function() {
-    before(function() {
+    const filename = 'test/tmp/test_cache.db';
+    const dbs = [];
+
+    const open = async (filename) => await new Promise((resolve, reject) => {
+        new sqlite3.cached.Database(filename, function (err) {
+            if (err != null) return reject(err);
+            resolve(this);
+        });
+    });
+    const close = async (db) => await util.promisify(db.close.bind(db))();
+
+    beforeEach(async function () {
+        dbs.length = 0;
         helper.ensureExists('test/tmp');
+        helper.deleteFile(filename);
     });
 
-    it('should cache Database objects while opening', function(done) {
-        var filename = 'test/tmp/test_cache.db';
+    afterEach(async function () {
+        await Promise.all(dbs.map(async (db) => await close(db)));
+        dbs.length = 0;
         helper.deleteFile(filename);
+    });
+
+    it('should cache Database objects while opening', function(done) {
         var opened1 = false, opened2 = false;
-        var db1 = new sqlite3.cached.Database(filename, function(err) {
+        dbs.push(new sqlite3.cached.Database(filename, function(err) {
             if (err) throw err;
             opened1 = true;
             if (opened1 && opened2) done();
-        });
-        var db2 = new sqlite3.cached.Database(filename, function(err) {
+        }));
+        dbs.push(new sqlite3.cached.Database(filename, function(err) {
             if (err) throw err;
             opened2 = true;
             if (opened1 && opened2) done();
-        });
-        assert.equal(db1, db2);
+        }));
+        assert.equal(dbs[0], dbs[1]);
     });
 
     it('should cache Database objects after they are open', function(done) {
-        var filename = 'test/tmp/test_cache2.db';
-        helper.deleteFile(filename);
-        var db1, db2;
-        db1 = new sqlite3.cached.Database(filename, function(err) {
+        dbs.push(new sqlite3.cached.Database(filename, function(err) {
             if (err) throw err;
             process.nextTick(function() {
-                db2 = new sqlite3.cached.Database(filename, function(err) {
+                dbs.push(new sqlite3.cached.Database(filename, function(err) {
+                    if (err) throw err;
                     done();
+                }));
+                assert.equal(dbs[0], dbs[1]);
+            });
+        }));
+    });
 
+    it('cached.Database() callback is called asynchronously', async function () {
+        await Promise.all([0, 1].map(() => new Promise((resolve, reject) => {
+            let callbackCalled = false;
+            dbs.push(new sqlite3.cached.Database(filename, (err) => {
+                callbackCalled = true;
+                if (err != null) return reject(err);
+                resolve();
+            }));
+            assert(!callbackCalled);
+        })));
+    });
+
+    it('cached.Database() callback is called with db as context', async function () {
+        await Promise.all([0, 1].map((i) => new Promise((resolve, reject) => {
+            dbs.push(new sqlite3.cached.Database(filename, function (err) {
+                if (err != null) return reject(err);
+                if (this !== dbs[i]) return reject(new Error('this !== dbs[i]'));
+                resolve();
+            }));
+        })));
+    });
+
+    it('db.close() callback is called asynchronously', async function () {
+        dbs.push(await open(filename));
+        dbs.push(await open(filename));
+        while (dbs.length > 0) {
+            await new Promise((resolve, reject) => {
+                let callbackCalled = false;
+                dbs.pop().close((err) => {
+                    callbackCalled = true;
+                    if (err != null) return reject(err);
+                    resolve();
                 });
-                assert.equal(db1, db2);
+                assert(!callbackCalled);
             });
-        });
+        }
+    });
+
+    it('db.close() callback is called with db as context', async function () {
+        dbs.push(await open(filename));
+        dbs.push(await open(filename));
+        while (dbs.length > 0) {
+            await new Promise((resolve, reject) => {
+                const db = dbs.pop();
+                db.close(function (err) {
+                    if (err) return reject(err);
+                    if (this !== db) return reject(new Error('this !== db'));
+                    resolve();
+                });
+            });
+        }
+    });
+
+    it('db.close() does not close other copies', async function () {
+        dbs.push(await open(filename));
+        dbs.push(await open(filename));
+        await close(dbs.pop());
+        assert(dbs[0].open);
+    });
+
+    it('db.close() closes the underlying Database after closing the last copy', async function () {
+        dbs.push(await open(filename));
+        dbs.push(await open(filename));
+        const db = dbs[0];
+        await close(dbs.pop());
+        await close(dbs.pop());
+        assert(!db.open);
+    });
+
+    it('cached.Database() returns an open Database after closing', async function () {
+        dbs.push(await open(filename));
+        await close(dbs.pop());
+        dbs.push(await open(filename));
+        assert(dbs[0].open);
     });
 });