PGresult* leak in `pg_st_deallocate_statement()` when using `PQclosePrepared` (libpq >= 17)

[rorsarach]

Background

While investigating a memory growth issue in a derivative PostgreSQL driver based on DBD::Pg 3.10.x, we traced it to a lifecycle bug in dbd_st_destroy() and dbd_st_finish(). After reviewing DBD::Pg 3.20.0 source code, the SQL DEALLOCATE path appears to be fixed by the pointer comparison in dbd_st_destroy(). However, the PQclosePrepared() path introduced in 3.20.0 for libpq >= 17 has the same class of bug.

This report covers the PQclosePrepared path in DBD::Pg 3.20.0.

What Happens

When pg_st_deallocate_statement() takes the PQclosePrepared branch, it does:

/* dbdimp.c line 4205 */
imp_dbh->last_result = imp_sth->result
    = PQclosePrepared(imp_dbh->conn, imp_sth->prepare_name);
imp_dbh->result_clearable = DBDPG_FALSE;

At this point, both imp_sth->result and imp_dbh->last_result are overwritten with the result of PQclosePrepared(). The previous SELECT result (R_select) that was stored in those two pointers is lost — PQclear(R_select) is never called.

The reason R_select was still in those pointers: dbd_st_finish() in 3.20.0 does not touch imp_sth->result at all. It only handles async state and calls DBIc_ACTIVE_off(). So from execute time to destroy time, imp_sth->result keeps pointing to the SELECT result.

Step-by-step

1. $sth->execute()

   Execute stores the result:
     imp_sth->result      = R_select
     imp_dbh->last_result = R_select
     result_clearable     = FALSE

2. All rows fetched.

   dbd_st_fetch() clears DBIc_ACTIVE after the last row.
   imp_sth->result still = R_select (finish() does not touch it).

3. $sth destroyed. prepared_by_us = TRUE, so dbd_st_destroy() calls
   pg_st_deallocate_statement().

4. Inside pg_st_deallocate_statement(), libpq >= 17 branch:

     imp_dbh->last_result = imp_sth->result
         = PQclosePrepared(conn, prepare_name);
     result_clearable = FALSE

   R_select has no remaining reference. PQclear was not called.

5. Back in dbd_st_destroy():

     if (imp_sth->result == imp_dbh->last_result) {   /* TRUE: both = R_close */
         imp_dbh->result_clearable = DBDPG_TRUE;       /* hand R_close to dbh */
     }

   R_close will eventually be freed. R_select is already gone.

Why the SQL DEALLOCATE Path Does Not Have This Problem

In the SQL DEALLOCATE branch, _result("DEALLOCATE ...") replaces imp_dbh->last_result with
R_dealloc. When dbd_st_destroy() then checks:

if (imp_sth->result == imp_dbh->last_result) { /* R_select != R_dealloc: FALSE */
    ...
} else {
    PQclear(imp_sth->result);  /* R_select freed here */
}

The pointer mismatch triggers PQclear(R_select). The fix that was applied to the DEALLOCATE
branch was not applied to the PQclosePrepared branch.

Affected Version

DBD::Pg 3.20.0 compiled with libpq >= 17 (i.e. PGLIBVERSION >= 170000).

The SQL DEALLOCATE branch (libpq < 17) is not affected.

Reproduction

Requires: DBD::Pg 3.20.0, PostgreSQL 17+ client library (libpq 17), PostgreSQL server.

#!/usr/bin/env perl
use strict;
use warnings;
use DBI;

# Adjust connection details as needed
my $dsn  = 'dbi:Pg:dbname=testdb;host=localhost';
my $user = 'testuser';
my $pass = 'testpass';

my $dbh = DBI->connect($dsn, $user, $pass, {
    RaiseError         => 1,
    PrintError         => 0,
    AutoCommit         => 0,
    pg_server_prepare  => 1,
    pg_switch_prepared => 1,   # force server-side prepare on first execute
});

# Create a table with a wide column so memory growth is visible
$dbh->do(q{
    CREATE TEMP TABLE leak_test (
        id   serial,
        body text
    )
});
$dbh->do(q{
    INSERT INTO leak_test (body)
    SELECT repeat('x', 4096) FROM generate_series(1, 200)
});
$dbh->commit;

my $pid = $$;

sub rss_kb {
    open my $fh, '<', "/proc/$pid/status" or return 0;
    while (<$fh>) { return (split)[1] if /^VmRSS/; }
    return 0;
}

printf "start  RSS=%d kB\n", rss_kb();

for my $i (1 .. 2000) {
    # my $sth is lexically scoped: goes out of scope at end of loop body,
    # triggering DESTROY -> pg_st_deallocate_statement -> PQclosePrepared
    my $sth = $dbh->prepare('SELECT body FROM leak_test WHERE id > ?');
    $sth->execute(0);

    # Fetch all rows; driver clears DBIc_ACTIVE after last row
    1 while $sth->fetchrow_arrayref;

    # $sth destroyed here — PQclosePrepared runs, R_select is leaked
    printf "loop=%-5d RSS=%d kB\n", $i, rss_kb() if $i % 400 == 0;
}

$dbh->commit;
$dbh->disconnect;

Expected output: RSS stabilizes after the first few hundred iterations.

Actual output (example with libpq 17, wide rows): RSS grows with each block of iterations,
proportional to the number of rows times row size.

Fix

Inside pg_st_deallocate_statement(), before the PQclosePrepared() assignment, handle the
existing result the same way dbd_st_destroy() already handles it in the DEALLOCATE branch:

#if PGLIBVERSION >= 170000

    /* Free any existing result before overwriting both pointers. */
    if (imp_sth->result) {
        if (imp_sth->result != imp_dbh->last_result) {
            PQclear(imp_sth->result);
        } else if (imp_dbh->result_clearable) {
            PQclear(imp_dbh->last_result);
            imp_dbh->last_result = NULL;
        }
        imp_sth->result = NULL;
    }

    imp_dbh->last_result = imp_sth->result
        = PQclosePrepared(imp_dbh->conn, imp_sth->prepare_name);
    imp_dbh->result_clearable = DBDPG_FALSE;

[esabol]

Thank you for the detailed report, @rorsarach. Would you be willing to submit a PR?

[rorsarach]

Thank you for the detailed report, @rorsarach. Would you be willing to submit a PR?

I'm more than happy to, but this bug was found on version 3.10, consider maintaining it on this version？