diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc index 87cd52d7283f71..049fd786126fce 100644 --- a/src/node_sqlite.cc +++ b/src/node_sqlite.cc @@ -941,6 +941,19 @@ bool DatabaseSync::Open() { return false; } + // sqlite3_open_v2() assigns a database handle even when it fails. Such a + // handle is in a "sick" state and may only be used to retrieve the error + // and must then be released with sqlite3_close(). Close and reset the + // handle on any failure below so that a failed open() does not leave the + // database in an open state. + bool opened = false; + auto reset_connection_on_failure = OnScopeLeave([&]() { + if (!opened && connection_ != nullptr) { + sqlite3_close_v2(connection_); + connection_ = nullptr; + } + }); + // TODO(cjihrig): Support additional flags. int default_flags = SQLITE_OPEN_URI; int flags = open_config_.get_read_only() @@ -1003,6 +1016,7 @@ bool DatabaseSync::Open() { env()->isolate(), this, load_extension_ret, SQLITE_OK, false); } + opened = true; return true; } diff --git a/test/parallel/test-sqlite-database-sync.js b/test/parallel/test-sqlite-database-sync.js index ac3a3c66d6469a..af7677a3cfc037 100644 --- a/test/parallel/test-sqlite-database-sync.js +++ b/test/parallel/test-sqlite-database-sync.js @@ -2,7 +2,7 @@ const { skipIfSQLiteMissing } = require('../common'); skipIfSQLiteMissing(); const tmpdir = require('../common/tmpdir'); -const { existsSync } = require('node:fs'); +const { existsSync, mkdirSync } = require('node:fs'); const { join } = require('node:path'); const { DatabaseSync, StatementSync } = require('node:sqlite'); const { suite, test } = require('node:test'); @@ -308,6 +308,42 @@ suite('DatabaseSync.prototype.open()', () => { }); t.assert.strictEqual(db.isOpen, true); }); + + test('does not leave the database open after a failed open', (t) => { + // Regression test for https://github.com/nodejs/node/issues/63831 + const dbDir = join(tmpdir.path, `database-dir-${cnt++}`); + const dbPath = join(dbDir, 'failed-open.db'); + using db = new DatabaseSync(dbPath, { open: false }); + + // The directory does not exist, so opening the database fails. + t.assert.throws(() => { + db.open(); + }, { + code: 'ERR_SQLITE_ERROR', + message: /unable to open database file/, + }); + t.assert.strictEqual(db.isOpen, false); + + // The connection must not be usable after a failed open. + t.assert.throws(() => { + db.exec('SELECT 1'); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + t.assert.throws(() => { + db.function('fn', () => {}); + }, { + code: 'ERR_INVALID_STATE', + message: /database is not open/, + }); + + // The database can be opened once the underlying problem is resolved. + mkdirSync(dbDir); + t.assert.strictEqual(db.open(), undefined); + t.assert.strictEqual(db.isOpen, true); + db.exec('CREATE TABLE foo (id INTEGER PRIMARY KEY)'); + }); }); suite('DatabaseSync.prototype.close()', () => {