Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Provide error context when an error is thrown within WaitOnLock().

  1. [PATCH] Free memory allocated by waitonlock_error_callback()

    Aleksander Alekseev <aleksander@tigerdata.com> — 2025-09-19T11:16:46Z

    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

  2. Re: [PATCH] Free memory allocated by waitonlock_error_callback()

    Quan Zongliang <quanzongliang@yeah.net> — 2025-10-23T03:18:09Z

    
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 -0400
> 
>      Provide error context when an error is thrown within WaitOnLock().
> ```
> 

LGTM

Improve the completeness of the code.

  3. Re: [PATCH] Free memory allocated by waitonlock_error_callback()

    Michael Paquier <michael@paquier.xyz> — 2025-10-23T04:08:22Z

    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

  4. Re: [PATCH] Free memory allocated by waitonlock_error_callback()

    Tom Lane <tgl@sss.pgh.pa.us> — 2025-10-23T04:29:35Z

    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

  5. Re: [PATCH] Free memory allocated by waitonlock_error_callback()

    Aleksander Alekseev <aleksander@tigerdata.com> — 2025-10-23T13:32:56Z

    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