Skip to content

Clear PGresults from per-query rollback#188

Merged
davecramer merged 1 commit into
postgresql-interfaces:mainfrom
jarvis24young:fix-per-query-rollback-pqclear
May 28, 2026
Merged

Clear PGresults from per-query rollback#188
davecramer merged 1 commit into
postgresql-interfaces:mainfrom
jarvis24young:fix-per-query-rollback-pqclear

Conversation

@jarvis24young

@jarvis24young jarvis24young commented May 15, 2026

Copy link
Copy Markdown
Contributor

Problem

CC_internal_rollback() drains all PGresults returned by PQgetResult() in the PER_QUERY_ROLLBACK path, but did not clear each non-NULL result inside the loop. The command sent in this path contains two statements, ROLLBACK TO per_query_svp and RELEASE per_query_svp, so libpq can return more than one result. Overwriting pgres on the next iteration loses the previous result and leaks it.

Fix

Clear each PGresult immediately after handling it, matching the pattern already used by other PQgetResult() loops in connection.c.

Verification

Built the driver in WSL with ASan/UBSan and exercised the real unixODBC + psqlODBC + PostgreSQL path. The black-box reproducer used Protocol=7.4-2 and UseServerSidePrepare=1, prepared and executed a statement, ran DEALLOCATE ALL through ODBC, then freed the prepared ODBC statement. That makes the driver's internal DEALLOCATE fail, logs SAVEPOINT per_query_svp followed by ROLLBACK TO per_query_svp; RELEASE per_query_svp, and reaches CC_internal_rollback(PER_QUERY_ROLLBACK).

Before this patch, LSan reported: SUMMARY: AddressSanitizer: 432 byte(s) leaked in 2 allocation(s), with the stack PQgetResult -> CC_internal_rollback -> CC_send_query_append -> SC_set_prepared.

After this patch, the same reproducer exits with no LSan leak report. I also reran test/exe/error-rollback-test under the same ASan build; it passed.

@davecramer

Copy link
Copy Markdown
Contributor

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.

@davecramer davecramer merged commit 4fbb1eb into postgresql-interfaces:main May 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants