Repository navigation
Closed
Labels
Description
Problem
Database's constructor (src/database.cc) opens the connection and then creates the tables:
Database::Database(const char* path) : db_(nullptr) {
const int flags = SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_FULLMUTEX;
if (sqlite3_open_v2(path, &db_, flags, nullptr)) {
throw std::invalid_argument("Database could not be initialized"); // (a)
}
auto& reg = GetReflectionRegister();
for (const auto& contents : reg.records) {
const auto& record = contents.second;
CreateTableQuery query(db_, record);
query.Execute(); // (b)
}
}Two error paths leak the SQLite connection:
- (a) open failure:
sqlite3_open_v2allocates a connection handle even when it returns an error (except on an out-of-memory failure), and the SQLite docs require that handle to be passed tosqlite3_close. Heredb_is thrown away unclosed on the error branch. - (b) table-creation failure: if any
CreateTableQuery::Execute()throws, the constructor exits by exception. Because the object was never fully constructed, its destructor does not run, so the already-opendb_is leaked.Database::Initialize(src/database.cc) also never assignsinstance_in this case, so nothing else can close it either.
Impact
A failure during initialization leaks the SQLite connection for the process lifetime. This is the error-path counterpart to the normal-shutdown cleanup that already works.
Already addressed (was part of this issue)
- Null-check for
Instance()— implemented in the thread-safety work (Library is not thread-safe: shared connection, racy singleton, and non-atomic auto-increment #9 / PR Make Database thread-safe via serialized connection access (#9) #28):Instance()now throws a clear error when the database is not initialized, guarded by the singleton lifecycle mutex. - RAII destructor — also landed in Make Database thread-safe via serialized connection access (#9) #28:
Database::~Database()(src/database.cc) now closesdb_, so cleanup on the normal path no longer depends on a manualFinalize(). (The shared-ownership lifecycle relies on this: the connection is closed by~Databaseonce the lastInstance()handle is released.)
Remaining scope
Only the constructor error path is left: ensure db_ is closed if initialization fails, on both branches above. Suggested direction:
- Wrap the open handle in an RAII holder that owns
db_for the duration of the constructor body (so a throw during table creation closes it), or add atry/catcharound the table-creation loop that closesdb_and rethrows. - On the
sqlite3_open_v2failure branch, close the handle before throwing (the handle may be non-null even on failure).
Acceptance criteria
- A failure in
sqlite3_open_v2closes the (possibly non-null) handle before throwing — no leak. - A
CreateTableQuery::Execute()failure during construction closesdb_before the exception propagates — no leak, andinstance_remains unset. - The normal open → use →
Finalizepath is unchanged, and the existing lifecycle/thread-safety tests still pass. - A regression test exercises the table-creation failure path and asserts a clean error with no leaked connection (e.g. under a leak/ASan check, or via an injectable failure).
Activity
[-]Harden Database lifecycle: null-check Instance(), RAII connection close, no leak on init failure[/-][+]Harden Database lifecycle: RAII connection close, no leak on init failure[/+][-]Harden Database lifecycle: RAII connection close, no leak on init failure[/-][+]Harden Database lifecycle: no connection leak on init failure[/+]Close SQLite connection leak on Database init failure (#27) (#32)
jkalias commentedon Jul 8, 2026
Resolved in PR #32 (merged to
main).Database::Databasenow closesdb_on both init error paths: thesqlite3_open_v2failure branch callssqlite3_close(db_)before throwing, and the table-creation loop is wrapped in atry/catchthat closesdb_and rethrows (src/database.cc:84–105). Closing as completed.Generated by Claude Code