From c5c6170c2e9d4555f0fcb15afa6d7e47595572d0 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Mon, 8 Nov 2010 11:28:09 -0300 Subject: [PATCH] Fix permanent memory leak in autovacuum launcher get_database_list was uselessly allocating its output data, along some created along the way, in a permanent memory context. This didn't matter when autovacuum was a single, short-lived process, but now that the launcher is permanent, it shows up as a permanent leak. To fix, make get_database list allocate its output data in the caller's context, which is in charge of freeing it when appropriate; and the memory leaked by heap_beginscan et al is allocated in a throwaway transaction context. --- src/backend/postmaster/autovacuum.c | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index e717644..22cafd5 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -1754,6 +1754,9 @@ autovac_balance_cost(void) * get_database_list * Return a list of all databases found in pg_database. * + * The list and associated data is allocated in the caller's memory context, + * which is in charge of ensuring that it's properly cleaned up afterwards. + * * Note: this is the only function in which the autovacuum launcher uses a * transaction. Although we aren't attached to any particular database and * therefore can't access most catalogs, we do have enough infrastructure @@ -1766,6 +1769,10 @@ get_database_list(void) Relation rel; HeapScanDesc scan; HeapTuple tup; + MemoryContext cxt; + + /* This is the context that we will allocate our output data in */ + cxt = CurrentMemoryContext; /* * Start a transaction so we can access pg_database, and get a snapshot. @@ -1777,9 +1784,6 @@ get_database_list(void) StartTransactionCommand(); (void) GetTransactionSnapshot(); - /* Allocate our results in AutovacMemCxt, not transaction context */ - MemoryContextSwitchTo(AutovacMemCxt); - rel = heap_open(DatabaseRelationId, AccessShareLock); scan = heap_beginscan(rel, SnapshotNow, 0, NULL); @@ -1787,6 +1791,15 @@ get_database_list(void) { Form_pg_database pgdatabase = (Form_pg_database) GETSTRUCT(tup); avw_dbase *avdb; + MemoryContext oldcxt; + + /* + * Allocate our results in the caller's context, not the transaction's. + * We do this inside the loop, and restore the original context at the + * end, so that leaky things like heap_getnext() are not called in a + * potentially long-lived context. + */ + oldcxt = MemoryContextSwitchTo(cxt); avdb = (avw_dbase *) palloc(sizeof(avw_dbase)); @@ -1797,6 +1810,7 @@ get_database_list(void) avdb->adw_entry = NULL; dblist = lappend(dblist, avdb); + MemoryContextSwitchTo(oldcxt); } heap_endscan(scan); -- 1.7.1