Skip to content

Harden Database lifecycle: no connection leak on init failure #27

Closed

Description

@jkalias

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_v2 allocates 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 to sqlite3_close. Here db_ 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-open db_ is leaked. Database::Initialize (src/database.cc) also never assigns instance_ 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)

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 a try/catch around the table-creation loop that closes db_ and rethrows.
  • On the sqlite3_open_v2 failure branch, close the handle before throwing (the handle may be non-null even on failure).

Acceptance criteria

  • A failure in sqlite3_open_v2 closes the (possibly non-null) handle before throwing — no leak.
  • A CreateTableQuery::Execute() failure during construction closes db_ before the exception propagates — no leak, and instance_ remains unset.
  • The normal open → use → Finalize path 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

  1. changed the title [-]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[/+] on Jun 15, 2026
  2. changed the title [-]Harden Database lifecycle: RAII connection close, no leak on init failure[/-] [+]Harden Database lifecycle: no connection leak on init failure[/+] on Jul 7, 2026
  3. jkalias commented on Jul 8, 2026

    @jkalias
    OwnerAuthor

    Resolved in PR #32 (merged to main). Database::Database now closes db_ on both init error paths: the sqlite3_open_v2 failure branch calls sqlite3_close(db_) before throwing, and the table-creation loop is wrapped in a try/catch that closes db_ and rethrows (src/database.cc:84–105). Closing as completed.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions