Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit dbc6eb1

Browse files
committed
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.
1 parent 79e15c7 commit dbc6eb1

File tree

2 files changed

+14
-9
lines changed

2 files changed

+14
-9
lines changed

src/backend/storage/ipc/standby.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -865,16 +865,11 @@ LogStandbySnapshot(void)
865865

866866
/*
867867
* Get details of any AccessExclusiveLocks being held at the moment.
868-
*
869-
* XXX GetRunningTransactionLocks() currently holds a lock on all
870-
* partitions though it is possible to further optimise the locking. By
871-
* reference counting locks and storing the value on the ProcArray entry
872-
* for each backend we can easily tell if any locks need recording without
873-
* trying to acquire the partition locks and scanning the lock table.
874868
*/
875869
locks = GetRunningTransactionLocks(&nlocks);
876870
if (nlocks > 0)
877871
LogAccessExclusiveLocks(nlocks, locks);
872+
pfree(locks);
878873

879874
/*
880875
* Log details of all in-progress transactions. This should be the last

src/backend/storage/lmgr/lock.c

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3398,18 +3398,26 @@ GetLockStatusData(void)
33983398
}
33993399

34003400
/*
3401-
* Returns a list of currently held AccessExclusiveLocks, for use
3402-
* by GetRunningTransactionData().
3401+
* Returns a list of currently held AccessExclusiveLocks, for use by
3402+
* LogStandbySnapshot(). The result is a palloc'd array,
3403+
* with the number of elements returned into *nlocks.
3404+
*
3405+
* XXX This currently takes a lock on all partitions of the lock table,
3406+
* but it's possible to do better. By reference counting locks and storing
3407+
* the value in the ProcArray entry for each backend we could tell if any
3408+
* locks need recording without having to acquire the partition locks and
3409+
* scan the lock table. Whether that's worth the additional overhead
3410+
* is pretty dubious though.
34033411
*/
34043412
xl_standby_lock *
34053413
GetRunningTransactionLocks(int *nlocks)
34063414
{
3415+
xl_standby_lock *accessExclusiveLocks;
34073416
PROCLOCK *proclock;
34083417
HASH_SEQ_STATUS seqstat;
34093418
int i;
34103419
int index;
34113420
int els;
3412-
xl_standby_lock *accessExclusiveLocks;
34133421

34143422
/*
34153423
* Acquire lock on the entire shared lock data structure.
@@ -3467,6 +3475,8 @@ GetRunningTransactionLocks(int *nlocks)
34673475
}
34683476
}
34693477

3478+
Assert(index <= els);
3479+
34703480
/*
34713481
* And release locks. We do this in reverse order for two reasons: (1)
34723482
* Anyone else who needs more than one of the locks will be trying to lock

0 commit comments

Comments
 (0)