[PATCH] Free memory allocated by waitonlock_error_callback()
Hi,
Currently waitonlock_error_callback() allocates memory in ErrorContext
and doesn't explicitly free it. Valgrind is not happy about it and
generates multiple reports like this:
```
2,048 bytes in 2 blocks are definitely lost in loss record 1,097 of 1,165
at 0x999539: palloc (mcxt.c:1389)
by 0x9DCBFD: initStringInfoInternal (stringinfo.c:45)
by 0x9DCC9D: initStringInfo (stringinfo.c:99)
by 0x73A807: waitonlock_error_callback (lock.c:2027)
by 0x94E1DE: errfinish (elog.c:510)
by 0x750394: ProcSleep (proc.c:1614)
by 0x73A720: WaitOnLock (lock.c:1979)
by 0x7391C2: LockAcquireExtended (lock.c:1221)
by 0x738360: LockAcquire (lock.c:814)
by 0x741064: VirtualXactLock (lock.c:4844)
by 0x3E9D93: WaitForOlderSnapshots (indexcmds.c:492)
by 0x3F155B: ReindexRelationConcurrently (indexcmds.c:4216)
```
I propose adding pfree().
waitonlock_error_callback() was added in:
```
commit f727b63e810724c7187f38b2580b2915bdbc3c9c
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Aug 29 15:43:34 2025 -0400
Provide error context when an error is thrown within WaitOnLock().
```
--
Best regards,
Aleksander Alekseev
Attachments:
v1-0001-Free-memory-allocated-by-waitonlock_error_callbac.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Free-memory-allocated-by-waitonlock_error_callbac.patchDownload
From e98412e7b1133ece8c7003ee48c598fd27c8763f Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@tigerdata.com>
Date: Fri, 19 Sep 2025 13:06:35 +0300
Subject: [PATCH v1] Free memory allocated by waitonlock_error_callback()
---
src/backend/storage/lmgr/lock.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 4cc7f645c31..6c347eccc01 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -2030,6 +2030,8 @@ waitonlock_error_callback(void *arg)
errcontext("waiting for %s on %s",
GetLockmodeName(tag->locktag_lockmethodid, mode),
locktagbuf.data);
+
+ pfree(locktagbuf.data);
}
/*
--
2.43.0
On 9/19/25 7:16 PM, Aleksander Alekseev wrote:
Hi,
Currently waitonlock_error_callback() allocates memory in ErrorContext
and doesn't explicitly free it. Valgrind is not happy about it and
generates multiple reports like this:```
2,048 bytes in 2 blocks are definitely lost in loss record 1,097 of 1,165
at 0x999539: palloc (mcxt.c:1389)
by 0x9DCBFD: initStringInfoInternal (stringinfo.c:45)
by 0x9DCC9D: initStringInfo (stringinfo.c:99)
by 0x73A807: waitonlock_error_callback (lock.c:2027)
by 0x94E1DE: errfinish (elog.c:510)
by 0x750394: ProcSleep (proc.c:1614)
by 0x73A720: WaitOnLock (lock.c:1979)
by 0x7391C2: LockAcquireExtended (lock.c:1221)
by 0x738360: LockAcquire (lock.c:814)
by 0x741064: VirtualXactLock (lock.c:4844)
by 0x3E9D93: WaitForOlderSnapshots (indexcmds.c:492)
by 0x3F155B: ReindexRelationConcurrently (indexcmds.c:4216)
```I propose adding pfree().
waitonlock_error_callback() was added in:
```
commit f727b63e810724c7187f38b2580b2915bdbc3c9c
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri Aug 29 15:43:34 2025 -0400Provide error context when an error is thrown within WaitOnLock().
```
LGTM
Improve the completeness of the code.
On Thu, Oct 23, 2025 at 11:18:09AM +0800, Quan Zongliang wrote:
On 9/19/25 7:16 PM, Aleksander Alekseev wrote:
Currently waitonlock_error_callback() allocates memory in ErrorContext
and doesn't explicitly free it. Valgrind is not happy about it and
generates multiple reports like this:Improve the completeness of the code.
errfinish() calls MemoryContextReset() on ErrorContext so as any leaks
like the one you are cleaning up are taken care of. Still, what you
are suggesting is simple enough and silences a bit valgrind, so agreed
about the addition of this pfree().
Let's see if somebody objects to that.
--
Michael
Michael Paquier <michael@paquier.xyz> writes:
On Thu, Oct 23, 2025 at 11:18:09AM +0800, Quan Zongliang wrote:
Improve the completeness of the code.
errfinish() calls MemoryContextReset() on ErrorContext so as any leaks
like the one you are cleaning up are taken care of. Still, what you
are suggesting is simple enough and silences a bit valgrind, so agreed
about the addition of this pfree().
Let's see if somebody objects to that.
Yeah, I object. Didn't 89d57c1fb already solve this in a more
general way? If it's not fixed by that, why not?
regards, tom lane
Hi Tom,
Yeah, I object. Didn't 89d57c1fb already solve this in a more
general way? If it's not fixed by that, why not?
I can confirm that 89d57c1fb fixed the issue under question, thanks!
Marking the patch as Withdrawn.
--
Best regards,
Aleksander Alekseev