Re: Adding some error context for lock wait failures

Hi,

On 2025-10-09 11:22:39 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > valgrind complains that there's a memory leak here:
> 
> > ==374853== 1,024 bytes in 1 blocks are definitely lost in loss record 1,257 of 1,459
> > ==374853==    at 0xFD902A: palloc (mcxt.c:1389)
> > ==374853==    by 0x101A3D6: initStringInfoInternal (stringinfo.c:45)
> > ==374853==    by 0x101A46B: initStringInfo (stringinfo.c:99)
> > ==374853==    by 0xD8CF32: waitonlock_error_callback (lock.c:2027)
> > ==374853==    by 0xF916E2: errfinish (elog.c:510)
> 
> Hmm, that is interesting -- I'd expect error cleanup to deal with
> that.  Did you happen to notice the exact repro case?  It's surely
> easy enough to add a pfree, but I don't believe that other errcontext
> callbacks are any more careful than this one.

I think the difference to most other cases is that this is just an
informational message, so there simply isn't any error cleanup. It's possible
we should change that, as you say it's not hard to imagine other error
contexts called in < ERROR cases also leaking...


As for a repro, it seems the basic case of

A: CREATE TABLE foo();
A: BEGIN; LOCK TABLE foo;
B: BEGIN; LOCK TABLE foo;
A: SELECT pg_sleep(2);
A: COMMIT;
B: \c

triggers it.


Of course the backtrace is slightly different in that case compared to the
case that I reported upthread.

Greetings,

Andres Freund