Message-ID: <gh-postgresql-interfaces-psqlodbc-188-c4481070925@github.com>
From: "davecramer (@davecramer)" <noreply+davecramer@github.com>
To: "postgresql-interfaces/psqlodbc" <noreply+postgresql-interfaces-psqlodbc@github.com>
Date: Mon, 18 May 2026 19:12:38 +0000
Subject: Re: [postgresql-interfaces/psqlodbc] PR #188: Clear PGresults from per-query rollback
In-Reply-To: <gh-postgresql-interfaces-psqlodbc-188@github.com>
References: <gh-postgresql-interfaces-psqlodbc-188@github.com>
List-Id: <gh-postgresql-interfaces-psqlodbc.github.com>
X-GitHub-Author-Login: davecramer
X-GitHub-Comment-Id: 4481070925
X-GitHub-Comment-Type: issue_comment
X-GitHub-Issue: 188
X-GitHub-Repo: postgresql-interfaces/psqlodbc
X-GitHub-Type: comment
X-GitHub-Url: https://github.com/postgresql-interfaces/psqlodbc/pull/188#issuecomment-4481070925
Content-Type: text/plain; charset=utf-8

The Bug
  
  CC_internal_rollback() loops over PQgetResult() to drain all results from a multi-statement command (ROLLBACK TO
  per_query_svp; RELEASE per_query_svp). Each iteration overwrites pgres with the next result, but never calls
  PQclear() on the previous one — leaking it.
  
  The Fix
  
      default:
          handle_pgres_error(self, pgres, __FUNCTION__, NULL, !ret);
      }
  +   PQclear(pgres);
  +   pgres = NULL;
  }
  
  PQclear(pgres) inside the loop, immediately after the switch handles the result. pgres = NULL is defensive (prevents
  double-free if the loop logic changes).
  
  Assessment
  
  Correct. This is the standard libpq pattern — every non-NULL PQgetResult() must be paired with a PQclear(). The fix
  is placed after the switch statement so all cases (success, error) get cleared. PQclear(NULL) is a no-op per libpq
  docs, so the NULL assignment is safe but not strictly necessary.
  
  Verified by LSan. The PR description shows the leak was 432 bytes across 2 allocations (matching the two results
  from the two-statement command), and the leak disappears after the fix.
  
  No test added — the existing error-rollback-test exercises this path and passes. A dedicated leak test would require
  LSan integration in CI, which is out of scope.