From: | Thomas Munro <thomas(dot)munro(at)gmail(dot)com> |
---|---|
To: | Melanie Plageman <melanieplageman(at)gmail(dot)com> |
Cc: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tomas Vondra <tomas(at)vondra(dot)me>, Noah Misch <noah(at)leadboat(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Andrey M(dot) Borodin" <x4mmm(at)yandex-team(dot)ru> |
Subject: | Re: Confine vacuum skip logic to lazy_scan_skip |
Date: | 2025-02-14 22:54:17 |
Message-ID: | CA+hUKGJ84L0yFD2S05WeCOXmBgBcYb2S5wMmzyohcMJeCwgksw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Feb 15, 2025 at 10:50 AM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
> On Sat, Feb 15, 2025 at 7:30 AM Melanie Plageman
> <melanieplageman(at)gmail(dot)com> wrote:
> > Seems valgrind doesn't like this [1]. I'm looking into it.
>
> Melanie was able to reproduce this on her local valgrind and
> eventually we figured out that it's my fault. I put code into
> read_stream.c that calls wipe_mem(), thinking that that was our
> standard way of scribbling 0x7f on memory that you shouldn't access
> again until it's reused. I didn't realise that wipe_mem() also tells
> valgrind that the memory is now "no access". That makes sense for
> palloc/pfree because when that range is allocated again it'll clear
> that. The point is to help people discover that they have a dangling
> reference to per-buffer data after they advance to the next buffer,
> which wouldn't work because it's in a circular queue and could be
> overwritten any time after that.
Here's a patch. Is there a tidier way to write this?
It should probably be back-patched to 17, because external code might
use per-buffer data (obviously v17 core doesn't or skink would have
told us this sooner). It's not a good time to push to 17 today,
though. Push to master now to cheer skink up and 17 some time later
when the coast is clear, or just wait?
Attachment | Content-Type | Size |
---|---|---|
0001-Fix-explicit-valgrind-interaction-in-read_stream.c.patch | text/x-patch | 3.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2025-02-14 23:03:50 | Re: Confine vacuum skip logic to lazy_scan_skip |
Previous Message | Tom Lane | 2025-02-14 22:44:40 | Re: New buildfarm animals with FIPS mode enabled |