Re: Confine vacuum skip logic to lazy_scan_skip

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

In response to

Responses

Browse pgsql-hackers by date

  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