From 81fe138ba252987815506caa57719822709b730f Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 4 Jun 2013 14:58:52 -0400
Subject: [PATCH] Fix memory leak in LogStandbySnapshot().
The array allocated by GetRunningTransactionLocks() needs to be pfree'd
when we're done with it. Otherwise we leak some memory during each
checkpoint, if wal_level = hot_standby. This manifests as memory bloat
in the checkpointer process, or in bgwriter in versions before we made
the checkpointer separate.
Reported and fixed by Naoya Anzai. Back-patch to 9.0 where the issue
was introduced.
In passing, improve comments for GetRunningTransactionLocks(), and add
an Assert that we didn't overrun the palloc'd array.
---
src/backend/storage/ipc/standby.c | 7 +------
src/backend/storage/lmgr/lock.c | 16 +++++++++++++---
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 8b3b8331aa9..3ebbb702aa1 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -875,16 +875,11 @@ LogStandbySnapshot(void)
/*
* Get details of any AccessExclusiveLocks being held at the moment.
- *
- * XXX GetRunningTransactionLocks() currently holds a lock on all
- * partitions though it is possible to further optimise the locking. By
- * reference counting locks and storing the value on the ProcArray entry
- * for each backend we can easily tell if any locks need recording without
- * trying to acquire the partition locks and scanning the lock table.
*/
locks = GetRunningTransactionLocks(&nlocks);
if (nlocks > 0)
LogAccessExclusiveLocks(nlocks, locks);
+ pfree(locks);
/*
* Log details of all in-progress transactions. This should be the last
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index f107d2040bf..0d9a3b66051 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -3353,18 +3353,26 @@ GetLockStatusData(void)
}
/*
- * Returns a list of currently held AccessExclusiveLocks, for use
- * by GetRunningTransactionData().
+ * Returns a list of currently held AccessExclusiveLocks, for use by
+ * LogStandbySnapshot(). The result is a palloc'd array,
+ * with the number of elements returned into *nlocks.
+ *
+ * XXX This currently takes a lock on all partitions of the lock table,
+ * but it's possible to do better. By reference counting locks and storing
+ * the value in the ProcArray entry for each backend we could tell if any
+ * locks need recording without having to acquire the partition locks and
+ * scan the lock table. Whether that's worth the additional overhead
+ * is pretty dubious though.
*/
xl_standby_lock *
GetRunningTransactionLocks(int *nlocks)
{
+ xl_standby_lock *accessExclusiveLocks;
PROCLOCK *proclock;
HASH_SEQ_STATUS seqstat;
int i;
int index;
int els;
- xl_standby_lock *accessExclusiveLocks;
/*
* Acquire lock on the entire shared lock data structure.
@@ -3422,6 +3430,8 @@ GetRunningTransactionLocks(int *nlocks)
}
}
+ Assert(index <= els);
+
/*
* And release locks. We do this in reverse order for two reasons: (1)
* Anyone else who needs more than one of the locks will be trying to lock
--
GitLab